From ec8ff547b765ac93b455bfd720612008216347a9 Mon Sep 17 00:00:00 2001 From: Florian Winkler Date: Tue, 19 May 2026 14:07:17 +0200 Subject: [PATCH 1/2] Enforce viewer-aware evidence visibility in the citation resolver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolveCitations previously ignored the viewer context and turned every resolvable evidence reference into a visible citation. It now enforces the full access policy — workspace, status, region, required groups, internal visibility, and the evidence validity window. Sensitive text is redacted for viewers outside a redaction's cleared groups, citations group by source id, and unavailable references are counted without exposing their metadata. Adds 18 policy unit tests and fixes the vitest path alias so the suite loads on paths containing spaces. Co-Authored-By: Claude Opus 4.7 (1M context) --- SESSION.md | 51 ++++++++-- SUBMISSION.md | 47 ++++++++- lib/citations.ts | 87 ++++++++++++++-- test/citations-policy.test.ts | 186 ++++++++++++++++++++++++++++++++++ vitest.config.ts | 6 +- 5 files changed, 358 insertions(+), 19 deletions(-) create mode 100644 test/citations-policy.test.ts diff --git a/SESSION.md b/SESSION.md index 04c1519..611cca7 100644 --- a/SESSION.md +++ b/SESSION.md @@ -1,26 +1,63 @@ ## AI tools used +Claude Code (Anthropic) — used as an agentic assistant for repository +analysis, implementation, test authoring, and verification. ## Prompts and instructions -List the prompts or instructions you gave to AI tools during the challenge. +The assistant worked through a fixed, phase-gated process, with review and +approval required between phases: +1. Clone the challenge repository and analyse it in full. +2. Produce a written implementation plan and present it for review before + writing any code. +3. A follow-up instruction asked for a second, deeper review of the plan, + focused specifically on edge cases and pitfalls, before approval. +4. On approval, implement the plan. +5. Complete `SUBMISSION.md` and `SESSION.md`. -## AI outputs accepted +Standing instructions throughout: keep the fix small and focused, maintain +professional code quality, and keep the working tree clean. -Summarize the AI outputs, edits, commands, or recommendations you accepted. +## AI outputs accepted +- The analysis locating the defect: `resolveCitations` in `lib/citations.ts` + ignored the viewer context (`void context`) and turned every resolvable + evidence reference into a visible citation. +- The implementation plan: enforce the full visibility policy in the resolver + and leave the display components unchanged. +- The deeper edge-case findings, in particular: the unavailable count is per + evidence reference, not per distinct source; `canViewSource` returned early + for internal sources, which would let them bypass later gates; two distinct + sources share a title, so grouping must use the source id; the validity + check must compare `validFrom`/`validUntil` against the answer's `asOf`, + not the evidence `updatedAt`. +- The implementation of `lib/citations.ts`: completed `canViewSource` and + `canUseEvidence`, added `isEvidenceCurrent` and the redaction helper, and + made `resolveCitations` enforce the policy. +- The new test file `test/citations-policy.test.ts` (18 tests). +- A one-line fix in `vitest.config.ts` (`fileURLToPath` for the path alias) so + the test suite resolves on a directory path containing spaces. ## Manual work -Summarize the parts you wrote, changed, reviewed, or corrected manually. - +- Defined the phase-gated process and reviewed the plan and the deeper + edge-case analysis at each gate. +- Required the second, deeper pitfalls review before approving implementation. +- Reviewed the final diff, the test run, and the build output. ## Discarded AI output -Summarize any AI suggestions you intentionally did not use and why. +No assistant output was rejected outright. Two approaches were considered and +deliberately not adopted, since the brief warns against unproven extra policy: +- Treating a source's `classification: "sensitive"` as an exclusion rule — it + is not in the specified exclusion list, and every sensitive source in the + fixtures is already excluded by other rules. +- Re-ordering evidence by `confidence` — the retrieval layer already ranks + evidence, and the answer's evidence order must be preserved. ## Notes -Do not include secrets, access tokens, private credentials, or personal data. Redact anything sensitive before submission. +No secrets, access tokens, credentials, or personal data are included in this +submission. diff --git a/SUBMISSION.md b/SUBMISSION.md index 18d4dde..2fac6ee 100644 --- a/SUBMISSION.md +++ b/SUBMISSION.md @@ -1,12 +1,55 @@ ## What changed? +The defect was in `lib/citations.ts`: `resolveCitations` ignored the viewer +context and turned every resolvable evidence reference into a visible +citation. The fix enforces the full visibility policy in the resolver. The +display components were not changed — they already render only what the +resolver returns. + +**`lib/citations.ts`** + +- `canViewSource` — completed with region and required-group checks, and + restructured so each rule is an independent gate. Previously the + internal-visibility branch returned early; region and group checks now apply + to internal sources as well. +- `canUseEvidence` — now also rejects evidence outside its validity window via + the new `isEvidenceCurrent` helper, and takes the answer's `asOf` date. + Stale, future-dated, and expired evidence is excluded. +- `applyRedactions` (new helper) — replaces a redaction's text unless the + viewer belongs to one of its cleared groups. +- `resolveCitations` — resolves evidence by exact id, preserves the answer's + evidence order, groups visible evidence by source id (not by title, since + two distinct sources share a title), numbers citations by first-visible + order, and counts every unavailable reference without exposing its metadata. + +**`vitest.config.ts`** + +- The test path alias now resolves via `fileURLToPath` instead of + `URL.pathname`, so the suite loads on directories whose path contains spaces. + +**`test/citations-policy.test.ts`** (new) + +- 18 unit tests: one per exclusion category (workspace, status, region, + required group, internal visibility, stale, validity window, missing id) and + for redaction with and without the viewer's cleared group. ## What did you test? +- `npm test` — 24 tests pass: the 6 supplied candidate-facing tests and the 18 + new ones. +- `npm run typecheck` (`tsc --noEmit`) — no errors. +- `npm run build` — the production build succeeds. +- `npm run dev` — all three conversations were verified against the rendered + output: only viewer-safe evidence is shown, the restricted phrase is + redacted, the unavailable count is correct, the empty states are correct, + and no excluded source's title, excerpt, owner, or workspace appears in the + page. ## Session log -- [ ] `SESSION.md` is completed. - +- [x] `SESSION.md` is completed. ## Final commit SHA + +`PENDING` — local solution commit; confirm or replace with the final commit +SHA after pushing to the private candidate repository. diff --git a/lib/citations.ts b/lib/citations.ts index 3976e9e..abc3961 100644 --- a/lib/citations.ts +++ b/lib/citations.ts @@ -1,4 +1,11 @@ -import type { Answer, EvidenceChunk, EvidenceConfidence, Source, ViewerContext } from "@/data/types"; +import type { + Answer, + EvidenceChunk, + EvidenceConfidence, + EvidenceRedaction, + Source, + ViewerContext +} from "@/data/types"; export type ResolvedEvidence = { id: string; @@ -23,7 +30,18 @@ export type CitationResolution = { evidenceToCitationNumber: Record; }; +function isRegionVisible(source: Source, context: ViewerContext): boolean { + return source.region === "global" || source.region === context.region; +} + +function hasRequiredGroups(source: Source, context: ViewerContext): boolean { + return source.requiredGroups.every((group) => context.groups.includes(group)); +} + export function canViewSource(source: Source, context: ViewerContext): boolean { + // Each rule is an independent gate. The region and group checks must run + // before the internal-visibility result is returned, otherwise internal + // sources would bypass them. if (source.workspaceId !== context.workspaceId) { return false; } @@ -32,6 +50,14 @@ export function canViewSource(source: Source, context: ViewerContext): boolean { return false; } + if (!isRegionVisible(source, context)) { + return false; + } + + if (!hasRequiredGroups(source, context)) { + return false; + } + if (source.visibility === "internal") { return context.role === "admin" && context.isInternalEmployee; } @@ -39,42 +65,85 @@ export function canViewSource(source: Source, context: ViewerContext): boolean { return true; } +// Evidence is in window when the answer's as-of date is on or after validFrom +// and on or before validUntil. The fixtures use ISO YYYY-MM-DD dates, which +// order correctly as plain strings, so no Date parsing (or timezone) is needed. +export function isEvidenceCurrent(evidence: EvidenceChunk, asOf: string): boolean { + if (asOf < evidence.validFrom) { + return false; + } + + if (evidence.validUntil !== undefined && asOf > evidence.validUntil) { + return false; + } + + return true; +} + export function canUseEvidence( evidence: EvidenceChunk, source: Source, - context: ViewerContext + context: ViewerContext, + asOf: string ): boolean { if (evidence.status !== "verified") { return false; } + if (!isEvidenceCurrent(evidence, asOf)) { + return false; + } + return canViewSource(source, context); } +// A redaction's visibleToGroups is an allow-list: a viewer in any one of those +// groups sees the original text; everyone else sees the replacement. +function applyRedactions( + excerpt: string, + redactions: EvidenceRedaction[] | undefined, + context: ViewerContext +): string { + if (!redactions) { + return excerpt; + } + + return redactions.reduce((text, redaction) => { + const viewerCleared = redaction.visibleToGroups.some((group) => + context.groups.includes(group) + ); + + return viewerCleared ? text : text.split(redaction.text).join(redaction.replacement); + }, excerpt); +} + export function resolveCitations( answer: Answer, evidenceChunks: EvidenceChunk[], allSources: Source[], context: ViewerContext ): CitationResolution { - void context; - const evidenceById = new Map(evidenceChunks.map((evidence) => [evidence.id, evidence])); const sourcesById = new Map(allSources.map((source) => [source.id, source])); const citationBySourceId = new Map(); const citations: ResolvedCitation[] = []; const evidenceToCitationNumber: Record = {}; + let unavailableCount = 0; - // BUG: the legacy resolver trusts retrieval output and groups every matching - // evidence ID. It misses source policy, time windows, redactions, and unavailable counts. for (const reference of answer.evidence) { const evidence = evidenceById.get(reference.evidenceId); const source = evidence ? sourcesById.get(evidence.sourceId) : undefined; - if (!evidence || !source) { + if (!evidence || !source || !canUseEvidence(evidence, source, context, answer.asOf)) { + // Every reference that does not resolve to visible evidence is counted + // once — missing ids included — and nothing about it is exposed. + unavailableCount += 1; continue; } + // Group by source id, never by title: distinct sources can share a title. + // A citation number is assigned when a source first gains visible + // evidence, so numbering follows first-visible order. let citation = citationBySourceId.get(source.id); if (!citation) { @@ -93,12 +162,12 @@ export function resolveCitations( citation.evidence.push({ id: evidence.id, - excerpt: evidence.excerpt, + excerpt: applyRedactions(evidence.excerpt, evidence.redactions, context), updatedAt: evidence.updatedAt, confidence: reference.confidence }); evidenceToCitationNumber[evidence.id] = citation.citationNumber; } - return { citations, unavailableCount: 0, evidenceToCitationNumber }; + return { citations, unavailableCount, evidenceToCitationNumber }; } diff --git a/test/citations-policy.test.ts b/test/citations-policy.test.ts new file mode 100644 index 0000000..135ed6d --- /dev/null +++ b/test/citations-policy.test.ts @@ -0,0 +1,186 @@ +import { describe, expect, it } from "vitest"; + +import type { EvidenceChunk, Source, ViewerContext } from "@/data/types"; +import { conversations, evidenceChunks, sources, viewer } from "@/data/fixtures"; +import { + canUseEvidence, + canViewSource, + isEvidenceCurrent, + resolveCitations +} from "@/lib/citations"; + +const onboardingAnswer = conversations[0].answer; +const asOf = onboardingAnswer.asOf; + +function findSource(id: string): Source { + const source = sources.find((entry) => entry.id === id); + if (!source) { + throw new Error(`missing fixture source: ${id}`); + } + return source; +} + +function findEvidence(id: string): EvidenceChunk { + const evidence = evidenceChunks.find((entry) => entry.id === id); + if (!evidence) { + throw new Error(`missing fixture evidence: ${id}`); + } + return evidence; +} + +describe("canViewSource — source visibility gates", () => { + it("excludes sources from another workspace", () => { + expect(canViewSource(findSource("src_globex_onboarding"), viewer)).toBe(false); + }); + + it("excludes archived sources", () => { + expect(canViewSource(findSource("src_acme_archived_policy"), viewer)).toBe(false); + }); + + it("excludes sources when the viewer lacks a required group", () => { + expect(canViewSource(findSource("src_acme_security_playbook"), viewer)).toBe(false); + }); + + it("requires every group listed on the source", () => { + const twoGroupSource: Source = { + ...findSource("src_acme_onboarding"), + id: "src_test_two_groups", + requiredGroups: ["hr", "finance"] + }; + + expect(canViewSource(twoGroupSource, viewer)).toBe(false); + expect(canViewSource(twoGroupSource, { ...viewer, groups: ["hr", "finance"] })).toBe(true); + }); + + it("excludes out-of-region sources but allows global ones", () => { + const euSource: Source = { + ...findSource("src_acme_onboarding"), + id: "src_test_eu", + region: "eu" + }; + + expect(canViewSource(euSource, viewer)).toBe(false); + expect(canViewSource(findSource("src_acme_onboarding"), viewer)).toBe(true); + }); + + it("allows internal sources only for internal admins", () => { + const internalSource: Source = { + ...findSource("src_acme_onboarding"), + id: "src_test_internal", + visibility: "internal" + }; + + expect(canViewSource(internalSource, viewer)).toBe(false); + expect( + canViewSource(internalSource, { ...viewer, role: "admin", isInternalEmployee: true }) + ).toBe(true); + }); + + it("still applies the region gate to internal sources", () => { + const internalAdmin: ViewerContext = { + ...viewer, + role: "admin", + isInternalEmployee: true + }; + const internalEuSource: Source = { + ...findSource("src_acme_onboarding"), + id: "src_test_internal_eu", + visibility: "internal", + region: "eu" + }; + + expect(canViewSource(internalEuSource, internalAdmin)).toBe(false); + }); +}); + +describe("canUseEvidence — evidence gates", () => { + it("excludes stale evidence", () => { + const stale = findEvidence("ev_acme_stale_checklist"); + expect(canUseEvidence(stale, findSource(stale.sourceId), viewer, asOf)).toBe(false); + }); + + it("excludes future-dated evidence at the answer's as-of date", () => { + const embargoed = findEvidence("ev_acme_embargoed_benefits"); + expect(canUseEvidence(embargoed, findSource(embargoed.sourceId), viewer, asOf)).toBe(false); + }); + + it("admits the same evidence once its valid-from date has passed", () => { + const embargoed = findEvidence("ev_acme_embargoed_benefits"); + expect( + canUseEvidence(embargoed, findSource(embargoed.sourceId), viewer, "2026-06-01") + ).toBe(true); + }); +}); + +describe("isEvidenceCurrent — validity window", () => { + it("rejects evidence whose valid-from date is still in the future", () => { + expect(isEvidenceCurrent(findEvidence("ev_acme_embargoed_benefits"), asOf)).toBe(false); + }); + + it("rejects evidence past its valid-until date", () => { + const expired: EvidenceChunk = { + ...findEvidence("ev_acme_access_checklist"), + id: "ev_test_expired", + validUntil: "2025-12-31" + }; + + expect(isEvidenceCurrent(expired, asOf)).toBe(false); + }); + + it("accepts evidence inside its window", () => { + expect(isEvidenceCurrent(findEvidence("ev_acme_access_checklist"), asOf)).toBe(true); + }); +}); + +describe("resolveCitations — policy enforcement", () => { + it("counts a missing evidence id as unavailable without creating a citation", () => { + const resolution = resolveCitations( + { + id: "ans_test_missing", + asOf, + body: "Points at nothing {{ev_does_not_exist}}.", + evidence: [{ evidenceId: "ev_does_not_exist", confidence: "low" }] + }, + evidenceChunks, + sources, + viewer + ); + + expect(resolution.citations).toHaveLength(0); + expect(resolution.unavailableCount).toBe(1); + expect(resolution.evidenceToCitationNumber).toEqual({}); + }); + + it("counts every unavailable reference, not distinct sources", () => { + const resolution = resolveCitations(onboardingAnswer, evidenceChunks, sources, viewer); + expect(resolution.unavailableCount).toBe(8); + }); + + it("never maps excluded evidence to an inline citation number", () => { + const resolution = resolveCitations(onboardingAnswer, evidenceChunks, sources, viewer); + + expect(resolution.evidenceToCitationNumber).not.toHaveProperty("ev_acme_archived_policy"); + expect(resolution.evidenceToCitationNumber).not.toHaveProperty("ev_globex_onboarding"); + expect(resolution.evidenceToCitationNumber).not.toHaveProperty("ev_acme_internal_comp"); + }); + + it("redacts sensitive text when the viewer lacks the cleared group", () => { + const resolution = resolveCitations(onboardingAnswer, evidenceChunks, sources, viewer); + const redacted = resolution.citations + .flatMap((citation) => citation.evidence) + .find((evidence) => evidence.id === "ev_acme_redacted_access"); + + expect(redacted?.excerpt).toContain("[redacted credential]"); + expect(redacted?.excerpt).not.toContain("temporary root token"); + }); + + it("keeps sensitive text for a viewer in the cleared group", () => { + const securityViewer: ViewerContext = { ...viewer, groups: ["hr", "security"] }; + const resolution = resolveCitations(onboardingAnswer, evidenceChunks, sources, securityViewer); + const revealed = resolution.citations + .flatMap((citation) => citation.evidence) + .find((evidence) => evidence.id === "ev_acme_redacted_access"); + + expect(revealed?.excerpt).toContain("temporary root token"); + }); +}); diff --git a/vitest.config.ts b/vitest.config.ts index 6a94bd9..df1156f 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -1,3 +1,5 @@ +import { fileURLToPath } from "node:url"; + import { defineConfig } from "vitest/config"; export default defineConfig({ @@ -9,7 +11,9 @@ export default defineConfig({ }, resolve: { alias: { - "@": new URL(".", import.meta.url).pathname + // Decode the file URL to a native path; .pathname stays URL-encoded and + // fails to resolve on directories whose path contains spaces. + "@": fileURLToPath(new URL(".", import.meta.url)) } } }); From e92f62302d0677665da59b59feefe01be166a559 Mon Sep 17 00:00:00 2001 From: Florian Winkler Date: Tue, 19 May 2026 14:34:56 +0200 Subject: [PATCH 2/2] docs: record solution commit SHA in SUBMISSION.md Co-Authored-By: Claude Opus 4.7 (1M context) --- SUBMISSION.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/SUBMISSION.md b/SUBMISSION.md index 2fac6ee..865bed1 100644 --- a/SUBMISSION.md +++ b/SUBMISSION.md @@ -51,5 +51,7 @@ resolver returns. ## Final commit SHA -`PENDING` — local solution commit; confirm or replace with the final commit +`ec8ff547b765ac93b455bfd720612008216347a9` + +This is the local solution commit. Confirm or replace it with the final commit SHA after pushing to the private candidate repository.