From 822c36ee76b7c9ceea9d9fb4d8ef43088d802de2 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Jul 2026 01:06:59 +0000 Subject: [PATCH] fix: line-aware manifest diagnostics and token-collision detection 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 Claude-Session: https://claude.ai/code/session_01UxYFd1KhMHh7ZLytLz4Yg8 --- docs/architecture.md | 9 ++++ src/contract/generate.test.ts | 26 ++++++++++- src/contract/generate.ts | 81 ++++++++++++++++++++++++++--------- 3 files changed, 95 insertions(+), 21 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index 39632df..c3b092a 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -66,10 +66,19 @@ The v1 loader owns: - `${NAME}` and `${NAME:-fallback}` environment interpolation. - Schema normalization and rejection of unknown fields. - Identity, probe, assertion, severity, URL, and header validation. +- Expansion of the optional `matrix` block into ordinary v1 probes. - A longest-first redactor for interpolated and configured credential values. Contracts are reviewable security artifacts. Keep real credentials out of them even though the loader redacts known values from its own diagnostics. +### Matrix expansion + +A contract may declare a `matrix` block instead of (or alongside) explicit probes: identities, and for each source an identifier, a synthetic canary, and an `allow` list. Validation expands the identity × source cross-product deterministically — every unauthorized identity becomes a critical deny probe (`source_absent` + `not_contains` the canary) and every authorized identity becomes a medium positive control. Expansion happens entirely inside the loader, before the runner sees the contract, so adapters, assertions, reporters, severities, and exit codes are unchanged, and a violated cell reports the YAML position of its matrix source entry. + +### Access-manifest generator + +`contextfence generate` is a build-time codegen step, not a runtime component. It reads a connector-neutral access manifest (identities, sources, per-source `allow` lists — the shape an IdP or permissions export produces), derives probe keys and canaries deterministically when omitted, fails closed on unknown fields, duplicate keys, undefined identities, and identity ids whose derived credential placeholders collide, and emits a matrix contract with environment placeholders instead of secrets. The generated file is an ordinary contract: it earns no trust from having been generated and passes through the full loader on every run. + ### Probe runner The runner bounds concurrency and applies one aggregate timeout to each probe, including all of its setup requests and primary request. A probe may include setup requests—for example, priming a cache under one identity—before its primary request runs under the identity being tested. diff --git a/src/contract/generate.test.ts b/src/contract/generate.test.ts index 1c71e8b..0980120 100644 --- a/src/contract/generate.test.ts +++ b/src/contract/generate.test.ts @@ -55,7 +55,7 @@ describe("access manifest generator", () => { expect(loaded.contract.probes[0].category).toBe("matrix-deny"); }); - it("rejects an allow entry that references an undefined identity", () => { + it("rejects an undefined allow identity with a line-aware diagnostic", () => { const manifest = MANIFEST.replace("- finance-analyst", "- ghost"); try { generateBoundaryContract(manifest, {}, "manifest.yaml"); @@ -65,6 +65,30 @@ describe("access manifest generator", () => { const formatted = (error as BoundaryContractError).format(); expect(formatted).toContain("$.sources[0].allow[0]"); expect(formatted).toContain("UNKNOWN_IDENTITY"); + // The diagnostic must point at the offending allow entry, not 1:1. + const line = Number(formatted.match(/manifest\.yaml:(\d+):\d+/)?.[1]); + expect(line).toBeGreaterThan(10); + } + }); + + it("rejects identity ids that derive the same credential placeholder", () => { + const manifest = ` +identities: + - id: finance-analyst + - id: finance.analyst +sources: + - id: finance/plan.md + allow: + - finance-analyst +`; + try { + generateBoundaryContract(manifest, {}, "manifest.yaml"); + throw new Error("Expected generation to fail"); + } catch (error) { + expect(error).toBeInstanceOf(BoundaryContractError); + const formatted = (error as BoundaryContractError).format(); + expect(formatted).toContain("DUPLICATE_TOKEN_ENV"); + expect(formatted).toContain("CONTEXTFENCE_FINANCE_ANALYST_TOKEN"); } }); diff --git a/src/contract/generate.ts b/src/contract/generate.ts index 83686f9..7b00f16 100644 --- a/src/contract/generate.ts +++ b/src/contract/generate.ts @@ -63,6 +63,14 @@ function deriveCanary(key: string): string { } /** Parse and validate an access manifest, returning normalized identities and sources. */ +function pathText(path: readonly (string | number)[]): string { + return path.reduce( + (result, part) => + typeof part === "number" ? `${result}[${part}]` : `${result}.${part}`, + "$", + ); +} + function readManifest(source: string, sourceName: string): { name: string; prompt: string; @@ -78,10 +86,31 @@ function readManifest(source: string, sourceName: string): { schema: "core", }); const diagnostics: ContractDiagnostic[] = []; - const fail = (message: string, code: string, path: string, node?: unknown): void => { - const offset = (node as { range?: readonly number[] } | undefined)?.range?.[0]; - const position = typeof offset === "number" ? lineCounter.linePos(offset) : { line: 1, col: 1 }; - diagnostics.push({ path, message, code, line: position.line, column: position.col }); + const fail = (message: string, code: string, pathParts: readonly (string | number)[]): void => { + const path = [...pathParts]; + let position = { line: 1, col: 1 }; + while (path.length >= 0) { + let node: unknown; + try { + node = path.length === 0 ? document.contents : document.getIn(path, true); + } catch { + node = undefined; + } + const offset = (node as { range?: readonly number[] } | undefined)?.range?.[0]; + if (typeof offset === "number") { + position = lineCounter.linePos(offset); + break; + } + if (path.length === 0) break; + path.pop(); + } + diagnostics.push({ + path: pathText(pathParts), + message, + code, + line: position.line, + column: position.col, + }); }; if (document.errors.length > 0) { @@ -107,69 +136,81 @@ function readManifest(source: string, sourceName: string): { const known = new Set(["version", "name", "prompt", "identities", "sources"]); for (const key of Object.keys(root)) { - if (!known.has(key)) fail(`Unknown field ${key}.`, "UNKNOWN_FIELD", `$.${key}`); + if (!known.has(key)) fail(`Unknown field ${key}.`, "UNKNOWN_FIELD", [key]); } if (root.version !== undefined && root.version !== 1 && root.version !== "1") { - fail('Only access manifest version "1" is supported.', "UNSUPPORTED_VERSION", "$.version"); + fail('Only access manifest version "1" is supported.', "UNSUPPORTED_VERSION", ["version"]); } const name = typeof root.name === "string" && root.name.trim() ? root.name : "generated-permission-matrix"; const prompt = typeof root.prompt === "string" && root.prompt.trim() ? root.prompt : DEFAULT_PROMPT; const identities: ManifestIdentity[] = []; const identityIds = new Set(); + const tokenEnvOwners = new Map(); if (!Array.isArray(root.identities) || root.identities.length === 0) { - fail("identities must be a non-empty sequence.", "REQUIRED", "$.identities"); + fail("identities must be a non-empty sequence.", "REQUIRED", ["identities"]); } else if (root.identities.length > MAX_IDENTITIES) { - fail(`At most ${MAX_IDENTITIES} identities are allowed.`, "RESOURCE_LIMIT", "$.identities"); + fail(`At most ${MAX_IDENTITIES} identities are allowed.`, "RESOURCE_LIMIT", ["identities"]); } else { for (const [index, raw] of root.identities.entries()) { - const path = `$.identities[${index}]`; + const path = ["identities", index] as const; if (!isRecord(raw)) { fail("Each identity must be a mapping with an id.", "TYPE_MAPPING", path); continue; } const id = typeof raw.id === "string" ? raw.id : ""; if (!IDENTIFIER.test(id)) { - fail("identity id must be a valid identifier (alphanumeric start; letters, numbers, dot, underscore, colon, hyphen).", "INVALID_IDENTIFIER", `${path}.id`); + fail("identity id must be a valid identifier (alphanumeric start; letters, numbers, dot, underscore, colon, hyphen).", "INVALID_IDENTIFIER", [...path, "id"]); continue; } if (identityIds.has(id)) { - fail(`identity id ${id} is duplicated.`, "DUPLICATE_ID", `${path}.id`); + fail(`identity id ${id} is duplicated.`, "DUPLICATE_ID", [...path, "id"]); + continue; + } + const tokenEnv = envToken(id); + const owner = tokenEnvOwners.get(tokenEnv); + if (owner !== undefined) { + fail( + `identity ids ${owner} and ${id} both derive the credential placeholder ${tokenEnv}; rename one so each identity gets its own token.`, + "DUPLICATE_TOKEN_ENV", + [...path, "id"], + ); continue; } + tokenEnvOwners.set(tokenEnv, id); identityIds.add(id); const identityName = typeof raw.name === "string" && raw.name.trim() ? raw.name : id; - identities.push({ id, name: identityName, tokenEnv: envToken(id) }); + identities.push({ id, name: identityName, tokenEnv }); } } const sources: ManifestSource[] = []; const keys = new Set(); if (!Array.isArray(root.sources) || root.sources.length === 0) { - fail("sources must be a non-empty sequence.", "REQUIRED", "$.sources"); + fail("sources must be a non-empty sequence.", "REQUIRED", ["sources"]); } else if (root.sources.length > MAX_SOURCES) { - fail(`At most ${MAX_SOURCES} sources are allowed.`, "RESOURCE_LIMIT", "$.sources"); + fail(`At most ${MAX_SOURCES} sources are allowed.`, "RESOURCE_LIMIT", ["sources"]); } else { for (const [index, raw] of root.sources.entries()) { - const path = `$.sources[${index}]`; + const path = ["sources", index] as const; if (!isRecord(raw)) { fail("Each source must be a mapping with an id.", "TYPE_MAPPING", path); continue; } const id = typeof raw.id === "string" ? raw.id.trim() : ""; if (!id) { - fail("source id must be a non-empty string.", "TYPE_STRING", `${path}.id`); + fail("source id must be a non-empty string.", "TYPE_STRING", [...path, "id"]); continue; } let key: string; if (raw.key !== undefined) { if (typeof raw.key !== "string" || !IDENTIFIER.test(raw.key)) { - fail("source key must be a valid identifier.", "INVALID_IDENTIFIER", `${path}.key`); + fail("source key must be a valid identifier.", "INVALID_IDENTIFIER", [...path, "key"]); continue; } if (keys.has(raw.key)) { - fail(`source key ${raw.key} is duplicated.`, "DUPLICATE_ID", `${path}.key`); + fail(`source key ${raw.key} is duplicated.`, "DUPLICATE_ID", [...path, "key"]); continue; } keys.add(raw.key); @@ -182,11 +223,11 @@ function readManifest(source: string, sourceName: string): { const allow: string[] = []; if (raw.allow !== undefined) { if (!Array.isArray(raw.allow)) { - fail("allow must be a sequence of identity ids.", "TYPE_SEQUENCE", `${path}.allow`); + fail("allow must be a sequence of identity ids.", "TYPE_SEQUENCE", [...path, "allow"]); } else { for (const [allowIndex, allowed] of raw.allow.entries()) { if (typeof allowed !== "string" || !identityIds.has(allowed)) { - fail("allow references an identity that is not defined.", "UNKNOWN_IDENTITY", `${path}.allow[${allowIndex}]`); + fail("allow references an identity that is not defined.", "UNKNOWN_IDENTITY", [...path, "allow", allowIndex]); } else if (!allow.includes(allowed)) { allow.push(allowed); }