Skip to content

fix(raw): reject traversal segments when constructing raw content URLs - #3108

Merged
SamMorrowDrums merged 3 commits into
mainfrom
sammorrowdrums-raw-content-path-traversal-hardening
Aug 19, 2026
Merged

fix(raw): reject traversal segments when constructing raw content URLs#3108
SamMorrowDrums merged 3 commits into
mainfrom
sammorrowdrums-raw-content-path-traversal-hardening

Conversation

@SamMorrowDrums

Copy link
Copy Markdown
Collaborator

Summary

raw.Client builds raw.githubusercontent.com URLs with
url.URL.JoinPath(owner, repo, ref, path). JoinPath normalizes ..
segments before producing the final URL, so a path (or, in principle,
owner/repo/ref) containing enough parent-directory segments could
consume the already-joined components and rebind the outbound request
to a different owner, repository, or ref than the one identified by
the resource URI.

Change

  • Added rejectPathTraversal, which checks every /-separated segment
    of owner, repo, ref/sha, and path and rejects the request if
    any segment is, or percent-decodes to, "..".
  • refURL, commitURL, and URLFromOpts now return (string, error)
    so the check happens at URL-construction time; GetRawContent
    propagates the error instead of issuing the request.
  • Benign filenames that merely contain two dots (e.g. file..txt,
    ..hidden) are left untouched.

Tests

Added table-driven cases to pkg/raw/raw_test.go covering: normal and
nested paths, refs/heads/... behavior, benign double-dot filenames,
literal .. segments (including a full owner/repo/ref rebinding
attempt), and percent-encoded (%2e%2e, partially-encoded) traversal
segments.

Validation

  • script/lint — 0 issues
  • script/test — all packages pass

No MCP tool schemas changed, so toolsnaps/docs regeneration was not needed.

Fixes #3103

url.URL.JoinPath normalizes ".." segments before producing the final
URL. A path containing enough parent-directory segments could
therefore consume the owner, repo, and ref components already joined
onto the base URL, rebinding the raw.githubusercontent.com request to
a different owner/repository/ref than the caller specified.

Reject any owner, repo, ref/sha, or path component whose "/"-separated
segments are, or percent-decode to, ".." before building the URL.
Benign filenames such as "file..txt" or "..hidden" are unaffected.

URLFromOpts, refURL, and commitURL now return an error alongside the
URL string so this can be enforced at construction time; GetRawContent
propagates it. Adds table-driven tests covering normal, nested, and
benign double-dot paths as well as literal and percent-encoded
traversal attempts.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings August 19, 2026 12:11
@SamMorrowDrums
SamMorrowDrums requested a review from a team as a code owner August 19, 2026 12:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds traversal validation to raw-content URL construction to prevent repository/ref rebinding.

Changes:

  • Rejects literal and percent-decoded .. segments.
  • Propagates URL validation errors.
  • Adds traversal and benign-filename tests.
Show a summary per file
File Description
pkg/raw/raw.go Adds URL traversal validation and error propagation.
pkg/raw/raw_test.go Tests valid paths and traversal rejection.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread pkg/raw/raw.go
SamMorrowDrums and others added 2 commits August 19, 2026 14:48
…tors

rejectPathTraversal split components on literal "/" before checking
each segment, so a segment containing an encoded separator (e.g.
"%2e%2e%2fsecret.txt") decoded to "../secret.txt" instead of "..", and
the check never caught it. Percent-decoding a segment can therefore
introduce new "/"-separated subsegments that were invisible to the
original literal split.

Recursively re-split and re-check the decoded form whenever decoding
changes a segment, so a ".." revealed by one or more layers of
percent-decoding (including through an encoded separator, or
double-encoding) is rejected regardless of where it appears.

Add regression tests for encoded-separator traversal, encoded
separators in other components, and double percent-encoded dot-dot
segments, plus a benign percent-encoded filename case to confirm
non-traversal decodes still pass through.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@SamMorrowDrums
SamMorrowDrums merged commit c195fb9 into main Aug 19, 2026
19 checks passed
@SamMorrowDrums
SamMorrowDrums deleted the sammorrowdrums-raw-content-path-traversal-hardening branch August 19, 2026 13:22
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.

Reject traversal segments when constructing raw content URLs

2 participants