Repository navigation
Conversation
Three related hardening fixes to api/repository.py's Repo class, all on the caller-controlled repo_url trust boundary: - save_path no longer trusts a local repo_url unchanged. Repo.is_local treats any non-URL string as a local path with no containment, so repo_url=/ made save_path return / unchanged, letting /codemap/file and the RAG indexer read/embed any server-readable file. save_path now resolves the path and requires it stay under an allowlisted root (the deepwiki data root by default, extendable via DEEPWIKI_ALLOWED_LOCAL_ROOTS), rejecting both out-of-root paths and symlink escapes from an allowed root. - download() now resolves the remote host and rejects private, loopback, link-local, multicast, reserved, and unspecified addresses before git ever connects. The previous URL check only constrained the scheme (http/https/ftp), not the host, so a repo_url could point the server's own git clone at an internal service regardless of the labeled repo_type. - _extract_repo_name now appends a digest of the full canonical identity (type + host + path) to the derived storage key. The previous key was only the last two URL path segments, so two different repositories (different host, or a GitLab subgroup path sharing its last two segments with another repo) could collide on the same clone directory and embedding index. Adds regression tests for all three in tests/test_repository.py and tests/test_extract_repo_name.py, including fail-before coverage run against the unpatched code.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three related hardening fixes to
api/repository.py'sRepoclass, all on the caller-controlledrepo_urltrust boundary:save_pathno longer trusts a localrepo_urlunchanged.Repo.is_localtreats any non-URL string as a local path with no containment, sorepo_url=/madesave_pathreturn/unchanged, letting/codemap/fileand the RAG indexer read/embed any server-readable file.save_pathnow resolves the path and requires it stay under an allowlisted root (the deepwiki data root by default, extendable viaDEEPWIKI_ALLOWED_LOCAL_ROOTS), rejecting both out-of-root paths and symlink escapes from an allowed root.download()now resolves the remote host and rejects private, loopback, link-local, multicast, reserved, and unspecified addresses before git ever connects. The previous URL check only constrained the scheme (http/https/ftp), not the host, so arepo_urlcould point the server's own git clone at an internal service regardless of the labeledrepo_type._extract_repo_namenow appends a digest of the full canonical identity (type + host + path) to the derived storage key. The previous key was only the last two URL path segments, so two different repositories (different host, or a GitLab subgroup path sharing its last two segments with another repo) could collide on the same clone directory and embedding index.Test plan
pytest tests/test_repository.py tests/test_extract_repo_name.py: new regression tests for all three issues pass.pytest tests/: full suite passes with no regressions (same 3 pre-existing, unrelated failures as main).api/repository.pyonly, re-ran).