fix(backend): skip symlinks instead of refusing the whole repository - #449
Merged
Merged
Conversation
A single symlink anywhere in a checkout rejected the entire import:
Repository contains a symlink, which is not supported.
{"path": "/tests/certs/valid/ca"}
That is psf/requests -- one link in a TLS test fixture, and one of the most
widely read Python repositories in existence could not be opened at all.
Symlinks are ordinary in real repositories: test fixtures, monorepo package
links, a docs path pointing at a shared file.
The parser now records each link and steps over it. Nothing about the
security posture weakens -- it strengthens. The original guard existed
because is_dir()/is_file()/stat() all follow symlinks, so walking one would
catalogue whatever it points at, including host filesystem content reached
through a link that escapes the checkout. Skipping never resolves the link at
all, so that content stays unreachable by construction rather than by a
check. The link's own target is deliberately never resolved or reported.
This also brings the file-tree walk in line with what the same parser already
does for metadata: _safe_file has always treated a symlinked manifest as
"simply absent rather than failing the whole import". The tree walk was the
only place that refused.
RepositoryMeta gains skipped_symlinks so the omission is visible rather than
silent -- a reader can tell "not followed" from "not present" -- which is the
same honest-limits posture the review layer takes for everything else it
cannot assess.
Verified against the repository that motivated this: psf/requests imports,
128 files, both links recorded, analysis completes and the snapshot seals.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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.
Closes #447.
The problem
One symlink anywhere in a checkout rejected the entire import:
{"code": "validation_error", "message": "Repository contains a symlink, which is not supported.", "details": {"path": "/tests/certs/valid/ca"}}That's
psf/requests— a link in a TLS test fixture, and one of the most widely read Python repositories in existence could not be opened at all. Symlinks are ordinary: test fixtures, monorepo package links, a docs path pointing at a shared file.The change
The parser records each link and steps over it.
The security posture strengthens rather than weakens. The original guard existed because
is_dir(),is_file()andstat()all follow symlinks, so walking one would catalogue whatever it points at — including host filesystem content reached through a link escaping the checkout. Skipping never resolves the link at all, so that content is unreachable by construction rather than by a check. The link's own target is deliberately never resolved or reported.It also brings the tree walk in line with what the same parser already does for metadata —
_safe_filehas always treated a symlinked manifest as "simply absent rather than failing the whole import". The tree walk was the only place that refused.RepositoryMetagainsskippedSymlinks, so the omission is visible rather than silent and a reader can tell not followed from not present — the same honest-limits posture the review layer takes everywhere else.Tests
The existing security tests are kept and made stricter. They previously asserted an exception was raised; they now assert the escape is unreachable — the link is absent from the tree, and nothing behind it appears anywhere in the response:
Plus new coverage for an internal link (not a second copy of its target), and for the
max_file_countpreflight walk (a skipped link counts for nothing against the budget).Verified against the repository that motivated it
psf/requests, live import:Contract
RepositoryMetagained a field, so the frontend contract is regenerated and five test fixtures that buildmetaby hand are completed. Backend suite green, frontend 457/457, ruff and eslint clean.