Skip to content

fix(core): reject traversal host segments to prevent cache path escape - #30

Open
detail-app[bot] wants to merge 1 commit into
devfrom
detail/bug-fix/fix-core-reject-traversal-host-segments-to-prevent-b8740a
Open

detail-app[bot] wants to merge 1 commit into
devfrom
detail/bug-fix/fix-core-reject-traversal-host-segments-to-prevent-b8740a

Conversation

@detail-app

@detail-app detail-app Bot commented Sep 12, 2026

Copy link
Copy Markdown

Detail bug report: View on Detail

Issue for this PR

Type of change

  • Bug fix

What does this PR do?

The safeHost validator in packages/core/src/repository.ts accepted "." and ".." as host values, even though the symmetric safeSegment validator already rejects them. Since host becomes the first path segment in cachePath, an input like "../log" produced a Reference with host === "..", making cachePath compute path.join(root, "..", "log") — escaping the repos cache root by one level. RepositoryCache.ensure then operated on that escaped path: recursively deleting a pre-existing non-git directory there and cloning a git repo into it. On a default install "../log" resolves to the app's own $XDG_DATA_HOME/opencode/log directory, which is created at startup.

The fix makes safeHost reject "." and ".." exactly like safeSegment does, closing the traversal at the parser boundary (the trust boundary all user input crosses). The same one-line change was applied to the byte-for-byte identical latent duplicate in packages/opencode/src/util/repository.ts.

Regression tests were added in both packages asserting that:

  • Traversal hosts ("../log", "./log", "..:owner/repo", ".:owner/repo", "git@..:owner/repo") are rejected at parse with the correct typed error.
  • Cache paths for every accepted reference form stay contained under the cache root.

How did you verify your code works?

  • Unit tests, typecheck (tsgo --noEmit), and lint (oxlint) all pass for both packages/core and packages/opencode. No new lint warnings were introduced (8 pre-existing consistent-return warnings on untouched functions are unchanged — confirmed via a baseline comparison with the changes stashed).
  • The new traversal-rejection and cache-path-containment tests pass in both packages.
  • Existing cache lifecycle integration tests (stale replacement, concurrent serialization, origin-mismatch replacement, branch isolation, enclosing-repo detection, typed error handling) remain green — no regression in cache behavior.
  • Verified at runtime that parseRemote("../log") now throws InvalidReferenceError where it previously returned host="..", and that all happy-path references (owner/repo, github.com/owner/repo, git@github.com:owner/repo.git, https://github.com/owner/repo.git, gitlab.com/group/repo) produce cache paths contained under the root. Also confirmed deeper traversal forms ("../../etc", "git@github.com:../../../etc/passwd") and file references are unaffected.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

Automatic Fixes PRs can be configured here.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants