Government-readiness review + Pass 1 hardening - #10
Merged
Conversation
Consolidated multi-track review assessing the system against high-assurance federal-sale criteria (SDVOSB → US government), covering evidence integrity, architecture correctness, the Python analytics engine, test/CI/reproducibility, federal compliance positioning, and new data-source expansion. Key verified findings: the SHA-256 replay hash covers row-count only (tampered and untampered data hash identically), the handoff verify script is a no-op, currency parsing is off by 1000x, and SECURITY.md contradicts the shipped code. Includes a prioritized remediation roadmap and a ranked list of new data sources (USAspending/SAM.gov demo stack, SAP change-document/audit-log forensic pack, Deltek Costpoint, Oracle Federal Financials, USSGL/GTAS conformance). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KP8RKHiEj57K4YVf7FRD2v
Replay hash: ProvenanceLogger.computeReplayHash passed Object.keys(result) as
JSON.stringify's replacer array, collapsing array-of-row results to [{},{}] so
the stored hash covered row count only and did not detect field tampering.
Add a shared recursive canonical serializer (canonicalStringify) in
provenance/replay.ts and route the logger's replay-hash and query-hash through
it. (verifyReplay still compares stored-to-recomputed rather than re-extracting
from source; that remains a separate follow-up.)
Currency: ecc_rfc mapSAPNumber did a blind comma->period replace, turning both
1.234,56 and 1,234.56 into 1.234 (1000x error); the Python csv_loader had the
same class of bug (1.234,56 -> 1.23456). Add a shared last-separator-wins parser
(parse-sap-number.ts) used by both TS adapters and a _parse_sap_float helper in
csv_loader.py.
Adds regression tests on both sides. Full suites green (mcp-server 1656 pass,
pattern-engine csv 21 pass).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KP8RKHiEj57K4YVf7FRD2v
SECURITY.md: correct verified overclaims — FI/CO tables are accessed by the FI/CO forensic tools (was listed under 'Data NOT Accessed'); remove fabricated audit-log session_id/client_ip fields; fix size-based (not 90-day) log retention; note the MCP server is stdio (no HTTP :3000) and the viewer has no built-in auth/TLS; recast SOC2/HIPAA/PCI/GDPR tables as 'supports the customer's program' rather than attested product controls; scope the redaction claim to the pattern engine's shareable mode; add an explicit authentication caveat. CLAUDE.md: replace phantom src/ dirs (evidence/, fi-co/, schema/) with the real tree (provenance/, extraction-registry/, schema-validator/, cross-system/, logging/, reports/, types/); fix env-var names (SAP_RFC_ASHOST, OLLAMA_HOST); mark the 9 evidence tools as defined-but-not-registered. docker-compose: remove the impossible HTTP :3000 healthcheck/port from the stdio MCP services; bind the viewer to 127.0.0.1 by default. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KP8RKHiEj57K4YVf7FRD2v
Hygiene: remove committed debris — agent1-work/ (four unrelated projects' docs), .agent3-staging/, .claude/context-checkpoint.md, and stale generated artifacts (output/pattern_cards.md, pattern-engine/test_output/). Extend .gitignore to keep these out and to ignore .env.rfc / .env.* (retaining *.example). Versions: reconcile mcp-server, viewer (1.0.1) and pattern-engine (0.1.1) to 3.0.1 to match the CHANGELOG; sync the npm lockfiles so 'npm ci' stays valid. CI: add .github/dependabot.yml (npm x2, pip, github-actions, docker), .nvmrc (20) and .python-version (3.11), and a non-blocking 'Security & SBOM' job running npm audit --omit=dev, pip-audit, and CycloneDX SBOM generation (uploaded as artifacts). Replace the hardcoded stale '605+ tests' CI summary line. CodeQL is already active via GitHub default setup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KP8RKHiEj57K4YVf7FRD2v
chrbailey
changed the title
chrbailey
marked this pull request as ready for review
July 5, 2026 20:43
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
2 participants
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 is
A multi-track government-readiness review (
docs/GOVERNMENT-READINESS-REVIEW.md) plus the first, safe slice of remediation it recommends. Scope was confirmed with the maintainer: honesty + hygiene + the two flagship correctness fixes, safe deletions only. Deeper architectural items (auth/TLS, wiring the 9 unregistered evidence tools, conformance/OCEL rewrites, real source-re-extraction replay, new data sources, FIPS profile) are explicitly deferred to follow-up PRs and tracked in the review doc's roadmap.Commits
ProvenanceLogger.computeReplayHashpassedObject.keys(result)asJSON.stringify's replacer array, so array-of-row results serialized to[{},{}]— the stored SHA-256 covered row count only and did not detect field tampering. Added a shared recursivecanonicalStringifyinprovenance/replay.tsand routed the logger's replay + query hashing through it. (verifyReplaystill compares stored-to-recomputed rather than re-extracting from source — that remains a separate follow-up; not overclaimed.)ecc_rfcmapSAPNumberdid a blind,→.replace, collapsing both1.234,56and1,234.56to1.234(1000× error); the Pythoncsv_loaderhad the same class of bug. Added a shared last-separator-wins parser used by both TS adapters and a Python equivalent.agent1-work/(four unrelated projects' docs),.agent3-staging/,.claude/context-checkpoint.md, and stale generated artifacts; gitignored.env.rfc; reconciled all versions to 3.0.1 (lockfiles synced); addeddependabot.yml,.nvmrc/.python-version, and a non-blocking Security & SBOM CI job.Verification
npm ci+npx tsc --noEmitclean; mcp-server 1,656 tests pass (was 1,639 — the +17 new regression tests), 4 skipped, 0 failing.make demoruns clean end-to-end and leaves the working tree clean.1.234,56/1,234.56/1234.56all parse to1234.56on both the TS and Python sides.Explicitly deferred (follow-up PRs)
Authentication/TLS on the MCP server and viewer; wiring the 9 evidence-infrastructure tools; comparator/conformance/OCEL correctness; tamper-evident (signed, hash-chained) audit logging and handoff-packet signing; real source-re-extraction replay; new data sources (USAspending/SAM.gov, SAP change-document/audit-log pack, etc.); FIPS deployment profile.
🤖 Generated with Claude Code
https://claude.ai/code/session_01KP8RKHiEj57K4YVf7FRD2v