Skip to content

Reject percent-encoded parent traversal in credential broker paths - #154

Open
yassine-ceo wants to merge 1 commit into
yc-software:mainfrom
yassine-ceo:fix/credential-broker-encoded-path-traversal
Open

Reject percent-encoded parent traversal in credential broker paths#154
yassine-ceo wants to merge 1 commit into
yc-software:mainfrom
yassine-ceo:fix/credential-broker-encoded-path-traversal

Conversation

@yassine-ceo

@yassine-ceo yassine-ceo commented Aug 3, 2026

Copy link
Copy Markdown

Closes the Git Broker finding from #125.

Problem

The credential broker and the git HTTP broker gate upstream paths through brokerPathAllowed, which matches the raw pathname against allowedPathPrefixes. Percent-encoded parent segments (..%2f, %2e%2e%2f) survive URL parsing with the encoded bytes intact, so a path like /acme/repo.git/..%2fadmin still starts with the allowed prefix while upstream servers decode it to /admin — a path traversal that bypasses the allowlist.

Fix

brokerPathAllowed now rejects any pathname containing a .. segment once percent-decoded (hasParentSegment), fail-closed on malformed escapes. This sits in the shared gate both brokerCredentialCall and brokerGitHttp flow through, so both entry points are covered by one check.

Tests

  • test/credential-broker.test.ts: encoded traversals (..%2f, %2e%2e%2f, %2e%2e%2f variants) refused with path_not_allowed, zero upstream calls.
  • test/git-http-broker.test.ts: encoded traversals in the git broker path refused with path_not_allowed, no upstream git fetch.

Verified: npm run typecheck, npm run lint (oxlint), and both affected test suites pass.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

@rajpratham1 rajpratham1 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.

The implementation addresses a legitimate path traversal concern, applies the validation consistently, and includes targeted regression tests that verify both rejection behavior and the absence of unintended upstream requests. I don't see any blocking issues in the proposed changes.

@16francej 16francej added code-pr Code submitted instead of an ADR bug Something isn't working security Security-sensitive change labels Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working code-pr Code submitted instead of an ADR security Security-sensitive change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants