feat(contract): Relayfile Path Contract v1 — grammar, fixtures, Go conformance runners#357
Conversation
…onformance runners Phase 1 of the path-contract stabilization plan (cloud/specs/relayfile-path-contract-stabilization.md): document and fence the scope-path grammar exactly as internal/httpapi/auth.go enforces it — suffix globs only, mid-path segment globs invalid. - contract/README.md: authoritative v1 grammar (canonical paths, normalization, scope grammar, glob matching, containment, mounts_at), flagged in-repo divergences, and the packages/local-mount/src/mount.ts path-theory audit (Appendix A). - contract/fixtures/*.json: language-neutral conformance fixtures. Invalid scope paths (incl. /github/repos/*/*/issues/**, the live hosted/OSS divergence) assert valid:false AND carry probe_files that must not match — an implementation that accepts a mid-path glob fails the suite even without a validity check. - internal/httpapi/path_contract.go: reference scopePathValid / scopePathMountRoot (grammar codification; no runtime verdict changes). - Fixture runners in internal/httpapi and internal/relayfile against the real scopeMatchesPath / scopePathMatches / normalizePath / withinBase. - make contract-test: named entry point (also runs under go test ./..., so existing ci.yml/contract.yml already gate it). Prior art: cloud#989 (HTTP-level contract suite; this is the path-vocabulary layer beneath it). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Warning Review limit reached
Next review available in: 23 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the Relayfile Path Contract v1, which defines path syntax, normalization, containment, scope-glob matching, and mount-root derivation. It adds language-neutral JSON fixtures, corresponding Go test runners, and a reference implementation for scope path validation and mount-root derivation. Feedback suggests adding a defensive check in scopePathMountRoot when slicing the prefix string to prevent potential panics if validation logic is bypassed or modified in the future.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| case strings.HasSuffix(scopePath, "*"): | ||
| prefix := strings.TrimSuffix(scopePath, "*") | ||
| return orRoot(prefix[:strings.LastIndex(prefix, "/")]), true |
There was a problem hiding this comment.
To prevent potential panics and adhere to defensive programming practices, it is safer to check if strings.LastIndex returns -1 before slicing the prefix string. Although scopePathValid currently guarantees that scopePath starts with / (and thus prefix contains at least one /), adding this check makes the code more robust against future changes or bypasses of the validation logic.
case strings.HasSuffix(scopePath, "*"):
prefix := strings.TrimSuffix(scopePath, "*")
idx := strings.LastIndex(prefix, "/")
if idx == -1 {
return "/", true
}
return orRoot(prefix[:idx]), true
Relayfile Eval ReviewRun: Passed: 4 | Needs human: 0 | Reviewable: 0 | Missing output: 0 | Failed: 0 | Skipped: 0 Human Review CasesNo reviewable human-review cases captured Relayfile output. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01f3fb9297
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 6. is compared **byte-exact and case-sensitive**. No Unicode normalization, | ||
| no percent-decoding at this layer (transport decoding happens before the |
There was a problem hiding this comment.
Canonical paths may currently end in whitespace because the grammar permits any non-/ byte, yet scopeMatchesPath applies strings.TrimSpace before authorization while normalizePath preserves that whitespace as part of the stored key. Consequently, an exact grant for /public also authorizes a distinct file such as /public on the file routes, contradicting the byte-exact guarantee here. Exclude trim-space boundaries from canonical paths or change the matching rule, and add a collision fixture rather than only testing harmless padding around a path.
Useful? React with 👍 / 👎.
Review finding on #357: "/" was declared valid with mounts_at "/" but no fixture pinned that it authorizes only the root node itself — the same valid-but-authorizes-nothing failure class the contract exists to eliminate. Adds root-scope-path-* auth-matching cases (exact-node match only; everything beneath root denied), a §5.1 note on the asymmetry, and a "stricter subsets" section: producers may accept a documented subset of the grammar, enforcers must implement all of it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Review addressed in bef155d: added |
1 participant
Summary
Phase 1 of the path-contract stabilization plan (
cloud/specs/relayfile-path-contract-stabilization.md). Per the operator ruling (2026-07-18), this repo owns the path contract; this PR documents and fences the grammar exactly asinternal/httpapi/auth.goalready enforces it. No runtime verdict changes — the fixture suite passes against unmodified enforcement code.contract/README.md— the authoritative v1 grammar: canonical file paths, normalization (quirks pinned + flagged), scope grammar (plane:resource:action[:path]), suffix-glob matching semantics, grant-set matching incl. narrow-grant precedence, containment, andmounts_atderivation. Mid-path segment globs are invalid:relayfile:fs:read:/github/repos/*/*/issues/**(the scope hosted cloud emits today) authorizes nothing under the contract.contract/fixtures/*.json— language-neutral conformance fixtures (path-normalization,containment,scope-paths,auth-matching; 58 cases). Invalid scope paths assertvalid: falseand carryprobe_filesthat must not match — so an implementation whose matcher accepts a mid-path glob fails the suite even if it skips validity checks. The live hosted/OSS divergence case is pinned verbatim.internal/httpapi/path_contract.go— referencescopePathValid/scopePathMountRoot(grammar codification only).internal/httpapiandinternal/relayfileexecute the fixtures against the realscopeMatchesPath/scopePathMatches/normalizePath/withinBase.make contract-test— named entry point; the suite also runs under plaingo test ./..., soci.yml/contract.ymlalready gate it.Also includes the Phase 1 audit of
packages/local-mount/src/mount.ts(README Appendix A): it holds three local-path theories (gitignore globs, ExcludeRules,normalizeRelativePosix) but never touches scope paths — no authorization exposure; the risk is vocabulary drift, flagged.Prior art: cloud#989 (HTTP-level contract-test spec) — that suite remains the layer above; this PR is the path-vocabulary layer it depends on.
Phase 2 consumers
relayfile-cloud,relayauth,relayfile-adapters,cloudrun these fixtures and report divergence (expected red: relayfile-cloud too permissive, cloud emits invalid scopes). No fixture may be edited to make an implementation pass.Test plan
make contract-test— green (60 httpapi subtests, 26 relayfile subtests)go test ./internal/httpapi ./internal/relayfile— greengo vet ./internal/...— clean🤖 Generated with Claude Code