fix: line-aware manifest diagnostics and token-collision detection#11
Merged
Merged
Conversation
Iterate on `contextfence generate` after a self-review pass: - Manifest diagnostics now resolve their YAML node position through the same path-walking lookup the contract parser uses, so an invalid allow entry reports its actual line and column instead of 1:1. - Two identity ids that slugify to the same credential placeholder (for example finance-analyst and finance.analyst both deriving CONTEXTFENCE_FINANCE_ANALYST_TOKEN) now fail closed with DUPLICATE_TOKEN_ENV instead of silently sharing one token, which would invalidate the boundary the generated grid is supposed to test. - Document the matrix expansion and access-manifest generator seams in docs/architecture.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UxYFd1KhMHh7ZLytLz4Yg8
devanchohan
force-pushed
the
claude/open-source-review-improve-iz3739
branch
from
July 17, 2026 01:07
fec02f6 to
822c36e
Compare
There was a problem hiding this comment.
Pull request overview
Adds a scalable authorization “matrix” abstraction to boundary contracts and a companion contextfence generate CLI command to produce matrix contracts from connector-neutral access manifests, while preserving the existing v1 probe runner and reporting semantics.
Changes:
- Extend the contract schema/loader to accept an optional
matrixblock and deterministically expand it into deny/allow probes with stable YAML locations. - Add
contextfence generateto convert an access manifest into a matrix contract with env placeholders for credentials and line-aware diagnostics. - Add new examples/docs and wire matrix fixtures + generator into release preflight.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/contract/schema.ts | Adds matrix validation + expansion into generated probes and tracks per-probe YAML paths. |
| src/contract/parser.ts | Uses probePaths to map generated probes back to matrix-source YAML locations. |
| src/contract/matrix.test.ts | Adds tests for matrix expansion semantics and location mapping. |
| src/contract/index.ts | Exports the new contract generator API. |
| src/contract/generate.ts | Implements access-manifest parsing/validation and deterministic matrix contract codegen. |
| src/contract/generate.test.ts | Adds generator tests (round-trip validation, derived keys/canaries, diagnostics). |
| src/cli/main.ts | Adds generate command execution path and help routing. |
| src/cli/args.ts | Adds generate command parsing and help text. |
| src/cli/args.test.ts | Adds CLI parsing tests for generate. |
| scripts/release-check.mjs | Runs matrix fixtures and generator in release preflight. |
| README.md | Documents matrix usage and the generate command. |
| package.json | Ensures examples/manifests is published. |
| examples/README.md | Adds example guidance for matrix expansion and generator usage. |
| examples/manifests/northstar-access.yaml | Adds a sample access manifest input for generation. |
| examples/contracts/matrix.boundary.yaml | Adds a live-template matrix contract example. |
| examples/contracts/matrix-leak.boundary.yaml | Adds a failing matrix “leak” fixture to demonstrate detection. |
| docs/architecture.md | Documents loader-owned matrix expansion and generator behavior. |
| CHANGELOG.md | Records new matrix block and contextfence generate additions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
What this changes
Follow-up hardening for
contextfence generateafter a self-review pass. (The matrix expansion and generate command themselves already landed onmainvia #9; this PR carries the remaining iteration on top.)generate's validation errors previously reported1:1for every finding. Diagnostics now resolve their YAML node position through the same path-walking lookup the boundary-contract parser uses, so an invalidallowentry reports its actual line and column.finance-analystandfinance.analystboth derivedCONTEXTFENCE_FINANCE_ANALYST_TOKENand would silently share one credential — invalidating the very boundary the generated grid is supposed to test. This now fails closed withDUPLICATE_TOKEN_ENV.Evidence
All data is synthetic. A manifest with an undefined
allowidentity now reports its exact position instead of1:1:A manifest with
finance-analystandfinance.analystfails withDUPLICATE_TOKEN_ENVnaming both ids and the shared placeholderCONTEXTFENCE_FINANCE_ANALYST_TOKEN.Verification
pnpm lintpnpm typecheckpnpm test(67 tests; regression tests added for both fixes, including asserting the diagnostic line is > 10)pnpm buildrelease-checkpreflight green).Safety and compatibility
DUPLICATE_TOKEN_ENVonly rejects manifests that were previously generating broken suites.CHANGELOG.md(covered by the existing Unreleasedgenerateentry).Screenshots or artifacts
Reproduce locally:
node dist/package/cli.js generate <manifest>with an undefinedallowidentity or colliding identity ids.🤖 Generated with Claude Code
https://claude.ai/code/session_01UxYFd1KhMHh7ZLytLz4Yg8