Match paths the way snyk-broker does - #131
Merged
Merged
Conversation
shawnburke
commented
Aug 27, 2026
shawnburke
marked this pull request as ready for review
August 27, 2026 00:19
shawnburke
force-pushed
the
grpc/accept-3-matching
branch
from
August 27, 2026 00:32
ff2856b to
592114d
Compare
shawnburke
force-pushed
the
grpc/accept-3-matching
branch
from
August 27, 2026 01:47
592114d to
f232be8
Compare
aszarama
approved these changes
Aug 27, 2026
shawnburke
force-pushed
the
grpc/accept-3-matching
branch
from
August 27, 2026 06:12
f232be8 to
77be223
Compare
keithfz
approved these changes
Aug 27, 2026
Rule matching diverged from the Node broker in ways that change which rule
answers a request, so a deployment moving onto the tunnel would route
differently on the file it is already running.
"*" now crosses "/", as it does in path-to-regexp, which the broker
compiles rules with. path.Match stops "*" at a separator, which made
accept.gitlab.json's "/*/info/refs" miss nested GitLab groups — the common
case — so a scaffolder clone that worked through the broker 404'd through
the tunnel. The mirror also holds now: "/api/*" no longer matches a bare
"/api", which the broker refuses.
${VAR} in a path is a segment placeholder, not a filter: it matches
whatever the caller sent there and the configured value is substituted
into the outgoing URL. That is a rewrite, and pinning the segment to the
configured value instead — which reads better as an allowlist — would have
404'd requests the broker rewrote and forwarded. Both spellings of the
path go through it, so the escaped form that travels keeps agreeing with
the decoded form the rule matched on, and a rule without a placeholder —
every rule Axon ships — costs nothing beyond the match that already
happened.
A rule with no method means GET. It used to match nothing, which made the
rule silently dead.
Directory traversal is refused and fragments are dropped before matching,
both of which the broker does and has tests for. Traversal is checked on
the decoded path, so an encoded "%2e%2e" cannot slip one past a rule that
would not have matched it spelled out.
The conformance runner grows expect.code so a fixture can say a request
was refused as malformed (400) rather than unmatched (404).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DFYsRuHoCbCBWEFvuaj9kV
The file lives under snykbroker/, which reads as though the broker path uses it. It does not: there the Node broker matches the rendered accept file itself and the agent only hands it the file. The one non-test caller is acceptfile.Router, which only the tunnel client builds. Worth stating in the file, since the parity work only makes sense once you know which path it governs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFYsRuHoCbCBWEFvuaj9kV
shawnburke
force-pushed
the
grpc/accept-3-matching
branch
from
August 27, 2026 21:03
77be223 to
d05add5
Compare
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.
Context
This is a stack of PRs to get the new gRPC tunnel up to spec with
snyk-broker--- with nosnyk-brokerin the picture we have to implement all of the accept file parsing and routing in Go.Specifically I had Claude audit all of the test cases in
snyk-broker, figure out which ones we care about, and make sure that the new tunnel supports them properly. In addition, this folds in the new wildcards support.This PR implements path matching in accordance with snyk-brokers code and test cases. Only affects grpc-tunnel pathways.
This discovered quite a few gaps and correctness issues in our handling here, some of which would have been problematic via the reflector had customers used it. In any case this aligns both and adds a security boundary around relative paths.
Claude Stuff
Stack 3/4 — based on #130. Review the top commit only.
Rule matching diverged from the Node broker in ways that change which rule answers a request, so a deployment moving onto the tunnel would route differently on the file it is already running. One of these breaks a shipped accept file.
*crosses/path-to-regexp@1.9.0, which the broker compiles rules with, turns*into(.*). Go'spath.Matchstops*at a separator./*/info/refs←/group/sub/p.git/info/refs/api/*←/apiaccept.gitlab.jsonships/*/info/refs,/*/git-upload-packand/*/git-receive-packfor scaffolder clones. Nested GitLab groups are the common case, and they 404'd over the tunnel while cloning fine through the broker.accept_file_gitlab_test.gocouldn't catch it: it asserts the shape of the JSON, never that a path matches.The second row is the mirror — the tunnel was more permissive than the allowlist author wrote.
${VAR}in a path is a rewrite, not a filterThe broker compiles it to a path-to-regexp param: it matches whatever the caller sent there and substitutes the configured value into the outgoing URL. So
/repos/${OWNER}/xaccepts/repos/evilcorp/xand forwards it as/repos/acme/x.Pinning the segment to the configured value reads better as an allowlist, and was what I wrote first — but it would have 404'd requests the broker rewrote and forwarded. The quirk comes with the migration.
Both spellings of the path go through the rewrite, so the escaped form that travels keeps agreeing with the decoded form the rule matched on. Guarded by a
strings.Contains(path, "${")check, so a rule without a placeholder — every rule Axon ships — costs nothing beyond the match that already happened.Smaller parity fixes
methodmeans GET, as in the broker. It used to match nothing, making the rule silently dead.%2e%2ecan't slip one past a rule that wouldn't have matched it spelled out.The conformance runner grows
expect.codeso a fixture can say a request was refused as malformed (400) rather than unmatched (404).The stack
🤖 Generated with Claude Code
https://claude.ai/code/session_01DFYsRuHoCbCBWEFvuaj9kV
Generated by Claude Code