Audit of main @5dfa6e4: 4 HIGH (S-1 secret-in-tree, M-1 missing migration path, C-1 unpropagated ID-mismatch fix in discover.py, D-1 stale ADR-F15 validation, T-1 untested resolution SQL) plus MED/LOW findings. Findings only; remediation planning deferred to the next loop phase. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
12 KiB
AUDIT — Development Loop 1 (main branch)
Auditor: Audit-Loop (Opus)
Date: 2026-06-15
Scope: entire main branch (HEAD 5dfa6e4)
Method: static review of source + schema + tests, branch-wide pattern scan
(ruff, mypy, defect-class greps), full test run (330 passed).
Status legend — HIGH = fix before relying on the feature · MED = fix soon, correctness/maintainability · LOW = polish · INFO = accepted risk / no action, documented for the record.
This document only records findings. Remediation planning is a separate step (next phase of the audit loop).
0. Headline
The most recent dev-loop work (F-15 citation graph + the follow-up fixes
cited_id→papers.id resolution and the paper_identifiers alias table) is
functionally green but unverified where it matters: the SQL that is the
fix has zero real-database coverage, the live-corpus validation in the ADR
predates the fix, and the same ID-mismatch bug the fix solves still lives in a
parallel code path (discover.py). Separately, a live DB password sits in an
un-ignored file and the new table has no migration path onto the live DB.
Test suite: 330 passed, ruff check clean, ruff format --check clean,
mypy clean on reviewed modules.
1. Security
S-1 — Live DB credential in an un-ignored file · HIGH
- Evidence:
.env.jetson-ingestcontainsDATABASE_URL=postgresql://researcher:05761b87…@localhost:5433/papers..gitignoreignores only the literal.env(line:# Environment secrets — NEVER commit→.env).git check-ignore .env.jetson-ingest→ NOT IGNORED. - Impact: one
git add -A && git commitleaks a real Postgres password to history. The file is currently untracked (??), so the leak has not happened yet — this is a near-miss, not an incident. - Note: the same gap exposes any future
.env.<profile>file.
S-2 — Non-constant-time token comparison (MCP HTTP) · LOW
- Evidence:
codex/mcp_server.py:340—if auth != f"Bearer {token}":. - Impact: theoretical timing side-channel on the Bearer token. Low on a
trusted LAN, but trivially fixable with
secrets.compare_digest.
S-3 — DNS-rebinding protection disabled · INFO (accepted)
- Evidence:
codex/mcp_server.py:35-37,TransportSecuritySettings(enable_dns_rebinding_protection=False)(commitd29dcf2). - Assessment: documented and mitigated — HTTP transport requires a Bearer
token (
_TokenAuth) and is firewalled to LAN IPs (UFW). A rebound browser request lacks the token. Residual risk only if the firewall assumption fails. No action; recorded so the trade-off stays visible.
2. Correctness
C-1 — discover.py still has the ID-format-mismatch bug F-15 fixed · HIGH
- Evidence:
codex/discover.py:20—WHERE cited_id NOT IN (SELECT id FROM papers); same pattern incocited_papers(discover.py:52-61),citing_papers,cited_by. - Root cause:
citations.cited_idis a mix of OpenAlex W-IDs (OpenAlex API path,ingest.py:229-231←sources/openalex.py:155) and DOIs/arXiv (GROBID/S2 paths).papers.idis the canonical DOI/arXiv. An ingested paper cited by its OpenAlex ID is not in(SELECT id FROM papers), so it is wrongly reported as a discovery lead. - Impact:
codex discover leads,codex discover cocited, and the MCPdiscover_leadstool over-report — foundational papers the user has already ingested show up as "cited but not ingested". This is exactly the D-05-class issue the F-15 graph path resolves via thepapers.openalex_idJOIN (graph.py:48-56) — but the fix was not propagated todiscover.py. Two divergent "dangling" implementations now disagree on what "ingested" means.
C-2 — paper_identifiers JOIN is effectively inert · MED
- Evidence:
graph.py:50-56second LEFT JOIN; populated only atingest.py:153-161, which inserts the samepaper.openalex_idthatpapers.openalex_idalready holds. - Impact: because ingest never records an alias that differs from
papers.openalex_id, thepiJOIN can only resolve what thepJOIN already resolves. It adds value only in the narrow case where a paper'sopenalex_idchanges across re-ingests (old alias retained viaON CONFLICT DO NOTHING). The table + unique index + seed INSERT + migration burden buy near-zero behaviour today — speculative generality. No test exercises an alias that diverges frompapers.openalex_id.
C-3 — MCP wiki_read returns the slug as its own "sources" · MED
- Evidence:
codex/mcp_server.py:138-149— comment says "Collect cited bibkeys", code setssources = [concept_slug]. - Impact: the
sourcesfield is a placeholder masquerading as data; any MCP client trusting it to list cited bibkeys gets the concept's own slug.
3. Documentation / Code Drift
D-1 — ADR-F15 validation predates the shipped graph · HIGH (integrity)
- Evidence: ADR-F15 (commit
a1a6f45) lands before the resolution fixes664906f(cited_id→papers.id) and5dfa6e4(paper_identifiers). - Stale claims:
- "Spike Result": 478 dangling nodes, specific PageRank scores, Bobenko/Springborn rank 7, 5.7 % spread — all measured on the pre-resolution (bipartite DOI→OpenAlex) graph. Post-fix, in-KB cited papers resolve to DOIs, so dangling count and PageRank distribution change.
- "Decision → Graph Construction": "inter-KB citations are structurally invisible" — now false.
- "Known Limitation: ID-Format Mismatch": "cross-citations… not represented" — now resolved by the code, contradicting the section.
- Impact: the GO decision rests on numbers the shipping code no longer produces. Validation must be re-run on the post-fix graph (or the ADR marked validation-pending).
D-2 — graph_cite_boost_alpha doc describes the fixed bug · MED
- Evidence:
config.py:296-300describesFinal score = dense_score * (1 + alpha * pagerank_score)— a multiply. Actual code (cli.py:107) divides distance:boosted = distance / (1.0 + alpha * pr_score). The ADR explicitly states the multiply form was the "critical bug corrected during the F-15 review gate". - Impact: the config docstring still documents the inverted (buggy) formula.
D-3 — paper_identifiers tagged "F-17" in schema, shipped as F-15 fix · LOW
- Evidence:
infra/schema.sql:129comment "F-17 Paper identifier aliases", but the table is introduced by the F-15 graph follow-up (5dfa6e4) and the ADR-F15 work. Feature-tag inconsistency; ADR-F15 doesn't mention the table at all.
4. Schema / Migration
M-1 — New table has no migration path onto the live DB · HIGH (operational)
- Evidence chain:
apply_schemadocstring (db.py:46-49) claims "all DDL statements use CREATE … IF NOT EXISTS" — false:papers,chunks,citations,code_linksand their indexes use bareCREATE TABLE/INDEX(schema.sql:16,33,56,68,30,41,…). Only F-09/F-16/F-17 DDL is idempotent.apply_schemais never called anywhere in the app — noinit/migrateCLI command (grep apply_schema→ onlydb.py+ a mocked unit test).ingest_all.shdoes not apply the schema; it only runscodex ingest.
- Impact: re-running
infra/schema.sqlagainst the existing Jetson DB fails on the firstCREATE TABLE papers(already exists) and rolls back the whole script in one implicit transaction — so the newpaper_identifierstable (and its seed INSERT) is never created on a DB that predates it.build_citation_graphthen fails withrelation "paper_identifiers" does not existon the live corpus. The migration step is currently manual and undocumented.
5. Test Integrity
T-1 — The F-15 resolution SQL has zero real coverage · HIGH
- Evidence: every test in
tests/graph/test_graph.pyandtests/graph/test_cli.pybuilds the graph from aMagicMockconn (_make_conn,_make_conn_with_paper_ids).test_cross_citation_uses_doi_when_in_kb(test_graph.py:83-99) pre-resolves the rows in Python and asserts the graph builds — it tests the mock, not theCOALESCE(p.id, pi.paper_id, …)+ two LEFT JOINs that are the fix. - Impact: the substance of the two most recent commits is unverified. 330
green tests give false confidence about the resolution logic, the
p.id IS NULLcorrelated guard, and thepaper_identifiersunique-index behaviour. Needs an integration test against a real (orpytest-managed) Postgres.
T-2 — apply_schema idempotency untested · MED
- Evidence:
tests/scaffold/test_db.py:24-32mocks the conn and only checksexecute/commitare called. The (false) idempotency claim in M-1 is never exercised.
6. Design / Usability
U-1 — graph report prints bare IDs, no titles · LOW
cli.py:582-588emitsscore <id>and bare dangling IDs — a mix of DOIs andW…OpenAlex IDs with no title/year. The ADR's readable author/title table is not what the CLI produces; output is hard to interpret.
U-2 — --cite-boost can only reorder within the top-limit page · LOW
cli.py:84-94appliesORDER BY … LIMIT limitin SQL before the boost (cli.py:96-109). A high-PR paper just outside the semantic cut can never be pulled in. With α=0.3 and PR≈0.002 the effect is ~0.06 % — effectively a within-page tie-breaker. Consistent with ADR intent, but worth stating as a limitation (the flag does far less than "weight results by PageRank" implies).
U-3 — graph report --json drops the small-corpus warning · LOW
cli.py:557-570returns before thegraph_min_corpus_sizewarning (cli.py:572-577). JSON consumers get neither the warning nor a flag for it.
U-4 — graph.find_co_cited implemented + tested but not wired to a CLI · LOW
graph.find_co_cited(graph.py:142) is unit-tested but exposed by nographsub-command (only the separate SQL-baseddiscover cocitedis wired). ADR-F15 R-43's co-citation is only half-surfaced.
7. State of Tree (INFO — no defect)
- The uncommitted diff (
wiki.py,synthesis.py,semanticscholar.py,tests/synthesis/test_cli.py,tests/wiki/test_compile.py) is a legitimateruff formatnormalization toline-length=100.ruff format --checkreports all 25 files formatted with these changes applied, meaning the committed code was non-compliant. These edits should be committed, not reverted. (Not a finding; clarifies the working-tree churn.)
8. Coverage of this audit (honesty ledger)
Deep-reviewed: graph.py, db.py, infra/schema.sql, ingest.py,
discover.py, provenance.py (scan), mcp_server.py, cli.py (search +
graph), config.py (graph settings), F-15 tests, ADR-F15; branch-wide pattern
scan (SQL injection, bare except, eval/exec/subprocess, secrets, timeouts).
Not yet deep-reviewed (no findings asserted):
synthesis.pygrounding guard / claim parser (_run_grounding_guard,_parse_claims) — the "zero-fact-leak (N-02)" core. Prompts reviewed; guard logic not yet traced.wiki.pygrounding guard + conflict detection (only the format diff seen).sources/(arxiv, openalex, semanticscholar) beyond citation shapes.parsing/(grobid, nougat, mathpix, figures, tex) — error handling inmathpix.py/figures.pyuses broadexcept Exceptionwithoutnoqa(mathpix.py:109,151,figures.py:125,152) — flagged for a later pass.embed.py,quality.py,models.py.
A second pass should close these before the audit is declared complete.