Promote Artifact Memory v0.1.3 candidate to main - #124
Conversation
…843acc9 Reconcile v0.1.2 main into dev
Close the M1 tamper-evidence gap from the corrected retrieval audit of 153f284: a two-step records_fts then records_fts_content forgery served inverted-index terms that passed content-row validation with the authentic source_record_set_digest. Every projection read now requires PRAGMA integrity_check to return ok inside _read_index, mapping any other result to a typed projection-unavailable failure. Security: forged search results can no longer pass validation carrying the authentic digest; detection relies on SQLite >= 3.44 FTS5 xIntegrity participation (verified on 3.52.0, cross-SQLite matrix still unverified). Compatibility: additive and fail-closed; no schema, CLI, receipt, or output-shape changes; clean projections unaffected at ~0.1 ms per query. Proves the seam with a synthetic two-step forgery fixture and a checked-in slice receipt replayed by run_conformance.sh. For issue #105; decision recorded in docs/decisions/0023.
Process the required-reviewer batch from PR #110 (Codex P1/P2, Qodo bugs 1-2; Sourcery independently flagged the first): - Reject runtimes whose PRAGMA integrity_check cannot verify the FTS5 inverted index (SQLite < 3.44) with a typed projection-unavailable: on an incapable runtime ok is absence of evidence, not verification. - Hold one read transaction across contract validation, integrity verification, and the caller query, so a concurrent writer cannot commit the two-step forgery between check and use. Security: closes the silent-bypass and check/use windows on the M0 gate. Compatibility: additive fail-closed; no schema, CLI, or receipt shape changes; clean projections unaffected. Synthetic regression fixtures cover both paths; slice receipt and contract/decision wording updated to match.
Gate projection reads on SQLite integrity verification (M0, #105)
Add search_receipt beside the unchanged search_records surface, per the corrected retrieval audit's Q1 step gated on the M0 integrity gate: the receipt (artifact-memory/search-receipt/v1) reports the raw query, matched record IDs, the projection's source_record_set_digest, and the integrity-gate outcome, pinning query evidence to the exact canonical record set as WITS 1151 requires. The receipt is issued inside the same gated read transaction that serves the query, so a tampered index yields a typed projection-unavailable failure instead of a vouched receipt. A search-receipt CLI command prints it human-readable or JSON. Both search surfaces share one query-failure classifier; behavior is identical. Security: query results become pinnable to the source record set with gate evidence attached; receipts cannot vouch for unverifiable indexes. Compatibility: additive only; search_records, related, provenance, and all pinned fixtures and receipts keep their shapes. Proven end to end with a synthetic slice receipt replayed by run_conformance.sh. For issue #106; decision recorded in docs/decisions/0024.
Add digest-bearing search receipts (Q1, #106)
feat: fix version
Implement Q2 of the corrected query-surface epic (audit findings F2 and F4): search and search-receipt accept --literal, which quotes the query as one FTS5 string with embedded double quotes doubled, so hyphens, colons, and bare operators match as content instead of being reinterpreted as query syntax (raw alpha-beta fails as column-filter syntax on SQLite 3.52.0; literal matches the adjacent phrase only). An empty literal query returns query-invalid without reaching SQLite. Query failures now classify on sqlite_errorcode & 0xff rather than SQLite message text: 1 is query-invalid, anything else is projection-unavailable. The contract now documents the query-surface boundary: search is lexically restricted to meaning.summary and labels, is an ungated confirmation oracle over that restricted meaning, applies no context-pack exclusion policy, and raw mode remains full FTS5 MATCH syntax by default. Receipts keep digesting the query exactly as typed in either mode. Security: removes a silent-reinterpretation footgun for agent-driven queries and drops the dependency on SQLite error wording. Compatibility: additive flag and defaulted keyword; no schema, receipt, or output-shape changes. Proven end to end with a synthetic slice replayed by run_conformance.sh. For issue #107; decision recorded in docs/decisions/0025.
Address the Sourcery finding on PR #114 (confirmed locally): the empty literal check ran inside the gated read, so a missing or invalid index returned projection-unavailable before the caller-input failure and the docstring's without-reaching-SQLite claim was wrong. Both search surfaces now evaluate the match expression before _read_index, so caller-input classification precedes any index access; regression tests pin the precedence against a missing index.
… and require an FTS5 records_fts Process the terminal required-reviewer batch from PR #114 (Codex P1/P2, Qodo): - Receipts record query_mode (raw|literal), added to the v1 search-receipt schema as an optional field so existing receipts stay valid while every new receipt identifies which grammar produced its results. - Literal mode now requires the query's own case-folded bytes in the indexed summary or labels on top of the FTS5 phrase match, so punctuation and spelling are significant: literal alpha-beta no longer matches adjacent 'alpha beta' text, while matching stays case-insensitive and single-term. - The projection contract requires records_fts to be an FTS5 virtual table; a regular table with the expected columns previously passed column and integrity checks and then misclassified its code-1 MATCH failure as query-invalid. It is now projection-unavailable at contract validation, before any MATCH executes. Validated on this head: 607 tests OK and run_conformance.sh OK including the regenerated search-literal slice (8 operations). For issue #107; decisions recorded in docs/decisions/0025.
Address the current-head Codex P2 on PR #114: the FTS5 declaration check searched the whole CREATE statement for 'using fts5', so a non-FTS5 virtual table could smuggle the marker in a comment (fts4 with /* using fts5 */ passed the gate and let receipts vouch for unsupported semantics). The contract now parses the module token immediately following USING and requires it to be exactly fts5; the regression test covers both the fts4-with-comment bypass and the plain regular table, and restores a genuine fts5 table between cases to prove reads recover.
Add literal search mode and error-code classification (Q2, #107)
Implement Q3 of the corrected query-surface epic (audit finding F5): search and search-receipt accept --exclude-superseded, dropping matches whose record lifecycle is superseded while the default keeps them as first-class hits. The filter runs inside the same gated read transaction as the match, composes with both query grammars, and search receipts record exclude_superseded beside query_mode and the query digest so every result-affecting parameter is bound and filtered results are replayable. This is a read-time lifecycle filter, not revocation; revocation suppression remains a projection-build input. Security: supersession becomes expressible from search and receipts state which mode ran, instead of superseded records being indistinguishable hits. Compatibility: additive flag, defaulted keyword, and optional receipt field; default behavior and all pinned fixtures unchanged. Proven end to end with a synthetic slice replayed by run_conformance.sh. For issue #108; decision recorded in docs/decisions/0026.
…hanged Process the current-head required-reviewer batch from PR #115 (Qodo, Codex P2): - Raw-mode exclusion now predicates lifecycle in SQL alongside the FTS match, and literal-mode exclusion bounds its lookups by the post-filtered candidates, so no filtered search materializes every superseded record ID or does work proportional to the projection. - Default search receipts omit exclude_superseded entirely, restoring the exact pre-filter v1 shape for consumers pinned to the earlier schema; only filtered receipts carry the field. Validated on this head: 610 tests OK and run_conformance.sh OK with the regenerated supersession slice. For issue #108; decisions recorded in docs/decisions/0026.
… exclusion Address the Sourcery security-lint thread on PR #115: replace the placeholder-count f-string IN query with one parameterized primary-key lifecycle lookup per post-filtered candidate. No search path builds SQL from string concatenation, and the work stays bounded by matched rows rather than the projection. Validated: 610 tests OK, run_conformance.sh OK.
Address the current-head Codex P2 on PR #115: the optional exclude_superseded property accepted false from external producers even though omission is the contract's only representation of the default, and such a receipt would also lose the pre-filter shape pinned v1 consumers expect. The property is now const true; a receipt carrying exclude_superseded: false fails schema validation. Validated: 610 tests OK, run_conformance.sh OK.
Add the exclude-superseded search filter (Q3, #108)
Implement Q4, the final step of the corrected query-surface epic (audit finding F3): search and search-receipt accept --rank, ordering results by FTS5 bm25 relevance with a deterministic record_id tiebreak; default order stays record_id alone. Per the second opinion's boundary ruling, ranked receipts carry a result_order label (bm25, record-id tiebreak, explicitly non-authoritative and corpus-dependent) as an optional v1 field, while default receipts omit it and keep the pre-ranking shape. Ranking composes with both query grammars and supersession exclusion inside the same gated read. The checked-in slice proves the boundary deterministically: bm25 inverts record_id order on a paired corpus, and adding three unrelated records flips the ranked order, so the corpus-dependence disclosure is evidenced, not asserted. bm25 cost and flip reachability at vault scale remain unmeasured open limitations, so the flag is not documented as stable. Security: no authority is claimed over order; ranking is a findability aid. Compatibility: additive flag, defaulted keyword, and optional receipt field; default output unchanged. For issue #109; decision recorded in docs/decisions/0027.
…ted flip Process the terminal required-reviewer batch from PR #116 (Codex P2, Qodo x2): - Rank with the explicit bm25(records_fts) function instead of the mutable rank alias: a persisted FTS5 rank configuration (bm25(0.0, 0.0)) can steer ORDER BY rank on an index that still passes contract validation and integrity_check, letting a tampered index receive a verified receipt for attacker-controlled weighting. The explicit function ignores the configuration; a regression test tampers a valid index and asserts the true order survives. - Supersession exclusion now composes in the same SQL statement for every mode combination, removing the per-result lifecycle queries (n+1) from ranked and literal paths. - The slice's corpus-growth flip now uses three lexically unrelated records (no query terms) with document-frequency asymmetry in the paired corpus, so the checked-in evidence substantiates the unrelated-growth claim it makes. Validated on this head: 615 tests OK and run_conformance.sh OK with the regenerated slice. For issue #109; decisions recorded in docs/decisions/0027.
Address the current-head Codex P2 on PR #116: the module-token check accepted a recreated records_fts with the same columns but altered indexing options (labels UNINDEXED, or a different tokenizer), which passes table_info, module, and integrity checks while silently steering bm25 and match semantics under a verified receipt. Contract validation now compares the normalized sqlite_master declaration against the canonical declaration extracted from the packaged projection contract itself, so the check cannot drift from the generator; the regression covers the labels-UNINDEXED and porter-tokenizer variants beside the earlier regular-table and fts4-with-comment cases. Validated: 615 tests OK, run_conformance.sh OK.
Add conditional bm25 ranking behind a flag (Q4, #109)
Close the four open limitations from the corrected query-surface epic record (issue #117): - Cross-SQLite matrix (scripts/run_cross_sqlite_matrix.py + cross_sqlite_probe.py): invariant-checking runner over local interpreters, sqlite3 CLI binaries, and pinned Docker images. Observed on 3.34.1 and 3.40.1 the runtime floor fails closed and the forgery is never served; on 3.46.1, 3.51.0, and 3.52.0 the forgery is detected typed and clean reads succeed; gate-passing runtimes agree on projection digests and default and ranked search results. - bm25 measurements (scripts/measure_ranked_search.py): ranked search is at cost parity with unranked (57.4 vs 62.6 ms at 1k; 291.3 vs 289.1 ms at 5k — revalidation dominates), and single-record additions caused no ranked-order flip in forty bounded trials; recorded in the performance baseline per decision 0015. - M1 deployment exploitability recorded in the threat model: the vector requires local write access to the generated projection file, the gate converts silent forgery into a typed failure, canonical records are unaffected, and v0 exposes no network surface to the projection. - ADRs 0023 and 0027 updated from unmeasured to measured with evidence pointers. Docker use is optional and read-only; the matrix degrades to local runtimes without it. For issue #117.
Process the terminal required-reviewer batch from PR #118 (Codex P1/P2 x2, Qodo x6, converging): - Determinism can no longer pass vacuously: at least two distinct gate-passing SQLite versions are required for invariants_hold, and the receipt reports gate-passing and fail-closed coverage explicitly. - Local probe failures are recorded as error entries and fail the matrix instead of silently vanishing. - Tier-B floor checks compare version tuples, not strings. - Docker execution binds to resolved image digests (recorded per entry), not mutable tags. - The probe adds literal-mode search and a logical-projection-snapshot digest to the cross-runtime fingerprint, so the determinism claim covers the generated projection, not just the pre-SQLite source digest. - Ranked measurements bind a generator profile and per-scale corpus digest, and flip trials use forty distinct single-record additions varied in length and term frequency rather than repeated identical text. Rerun evidence: matrix invariants_hold true across 3.34.1/3.40.1 (fail closed), 3.46.1/3.52.0 (gate passing, identical snapshot digests), and 3.51.0 CLI; measurements unchanged in substance (cost parity, 0/40 flips) and recorded with corpus digests. For issue #117.
…cile measurements Address the current-head Codex P2 batch on PR #118: - A Python or Docker runtime whose library tier is unavailable (import failure) now fails the matrix instead of being silently untested; only the intentionally tier-A-only CLI entries are exempt, marked library_tier_expected false. - A configured sqlite3 CLI whose version or FTS5 probe fails is recorded as an error entry and fails the matrix, matching local and Docker handling. - ADR 0027 now cites the recorded bound-receipt values (56.2/56.0 ms at 1k; 287.9/288.4 ms at 5k) with the generator profile and corpus-digest binding, removing the contradiction with the performance baseline. Validated: matrix invariants_hold true, 615 tests OK, run_conformance.sh OK. For issue #117.
Correct the prior commit's overstated validation claim: the preserved error entries exposed that the Homebrew sqlite3 CLI enables defensive mode by default and rejected the probe's shadow-table write, so the recorded run at d3f8a5e actually reported invariants_hold false for that entry — a real-latency class of failure the pre-hardening script silently dropped. The probe now disables defensive mode explicitly (it is a CLI configuration, not an engine capability) and both CLI binaries measure cleanly: six runtimes recorded, 3.34.1/3.40.1 fail-closed, 3.46.1/3.51.0/ 3.52.0 detecting, gate-passing versions agreeing on digests, snapshots, and default/literal/ranked results. 615 tests OK, run_conformance.sh OK. For issue #117.
Address the current-head Codex P2 pair on PR #118: the .dbconfig defensive-off dot-command echoes its state to stdout, which the CLI tier counted as an integrity row — confirmed on both local CLIs — so a below-3.44 CLI would have been misclassified as detecting the forgery; the echo is now filtered before interpretation. A Docker tag whose immutable digest cannot be resolved is a hard error entry instead of silently executing the mutable tag with no recorded identity. Matrix rerecorded: six runtimes, invariants hold, CLI integrity rows now contain only integrity_check output. For issue #117.
Verify the epic's still-unverified limitations with measured evidence (#117)
Harden projection integrity for v0.1.3
…ning Complete projection integrity follow-up hardening
Reviewer's GuidePromotes the dev branch’s v0.1.3 candidate to main, combining fail-closed SQLite/FTS5 projection integrity gates with additive digest-bearing search receipts, literal and supersession-aware search, optional non-authoritative BM25 ranking, updated release documentation, cross-runtime evidence, and expanded conformance tests. Sequence diagram for integrity-gated projection searchsequenceDiagram
participant Caller
participant Projection as projection._read_index
participant SQLite
participant Search as search_records
Caller->>Search: search_records(index_path, query)
Search->>Projection: _read_index(index_path)
Projection->>SQLite: _runtime_verifies_fts5_integrity()
SQLite-->>Projection: capability result
Projection->>SQLite: BEGIN
Projection->>SQLite: _validate_projection_contract()
Projection->>SQLite: PRAGMA integrity_check
alt integrity verified
Projection-->>Search: verified read transaction
Search->>SQLite: _matched_record_ids()
SQLite-->>Search: record IDs
Search-->>Caller: search results
else unavailable or tampered projection
Projection-->>Search: ValidationFailure(projection-unavailable)
Search-->>Caller: rejected result
end
Sequence diagram for digest-bearing search receiptssequenceDiagram
participant Caller
participant CLI as CLI
participant Receipt as search_receipt
participant Projection as _read_index
participant SQLite
Caller->>CLI: search-receipt index query --json
CLI->>Receipt: search_receipt(index, query)
Receipt->>Projection: _read_index(index)
Projection->>SQLite: _validate_projection_contract()
Projection->>SQLite: PRAGMA integrity_check
Projection-->>Receipt: verified snapshot
Receipt->>SQLite: _matched_record_ids()
Receipt->>SQLite: SELECT source_record_set_digest
Receipt-->>CLI: digest-bearing receipt
CLI-->>Caller: query_digest, record_ids, source_record_set_digest, integrity_gate
State diagram for SQLite projection availabilitystateDiagram-v2
[*] --> Unchecked
Unchecked --> CapabilityCheck: _runtime_verifies_fts5_integrity()
CapabilityCheck --> Unavailable: probe fails
CapabilityCheck --> ContractCheck: probe passes
ContractCheck --> Unavailable: schema or declaration mismatch
ContractCheck --> IntegrityCheck: contract valid
IntegrityCheck --> Unavailable: integrity_check != ok
IntegrityCheck --> Verified: integrity verified
Verified --> Reading: query in same read transaction
Reading --> Verified: read complete
Unavailable --> [*]
Verified --> [*]
Flow diagram for additive search modesflowchart LR
Query[Search query] --> Mode{Query mode}
Mode -->|raw| Raw[FTS5 MATCH syntax]
Mode -->|--literal| Literal[Quoted single literal term]
Raw --> Filter{--exclude-superseded?}
Literal --> Filter
Filter -->|yes| Lifecycle[Filter lifecycle != superseded]
Filter -->|no| Rank{--rank?}
Lifecycle --> Rank
Rank -->|yes| BM25["ORDER BY bm25(records_fts), record_id"]
Rank -->|no| IDs[ORDER BY record_id]
BM25 --> IDs
IDs --> Receipt{search-receipt?}
Receipt -->|yes| Evidence[Bind query digest and source record-set digest]
Receipt -->|no| Results[Return record IDs]
Evidence --> Results
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
PR Summary by QodoPromote Artifact Memory v0.1.3 retrieval and integrity hardening
AI Description
Diagram
High-Level Assessment
Files changed (69)
|
There was a problem hiding this comment.
Hey - I've found 3 security issues, and 1 other issue
Security issues:
- Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'. (link)
- Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'. (link)
- Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'. (link)
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="scripts/run_cross_sqlite_matrix.py" line_range="62-64" />
<code_context>
+ }
+ )
+ continue
+ if report["sqlite_version"] in seen:
+ continue
+ seen.add(report["sqlite_version"])
+ entries.append({"runtime": f"{name} ({path})", **report})
+ return entries
</code_context>
<issue_to_address>
**issue (bug_risk):** The matrix drops every later runtime whose reported SQLite version matches an earlier entry, even when the builds have different FTS5 capabilities, library availability, or result behavior. A capable build encountered first can therefore hide an incapable or divergent same-version build, allowing the matrix to report `invariants_hold` without testing every discovered runtime.
**Triggers:** When multiple Python interpreters, Docker images, or CLI binaries expose the same SQLite version but differ in compile-time features, linked-library behavior, or package availability.
**Suggested fix:** Deduplicate by an immutable runtime/build identity, such as executable path plus library/build fingerprint or Docker image digest, and retain separate entries for distinct runtimes even when their SQLite versions match.
</issue_to_address>
### Comment 2
<location path="scripts/run_cross_sqlite_matrix.py" line_range="50-52" />
<code_context>
completed = subprocess.run(
[path, str(PROBE)], capture_output=True, text=True, check=True, timeout=180
)
</code_context>
<issue_to_address>
**security (python.lang.security.audit.dangerous-subprocess-use-audit):** Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.
*Source: opengrep*
</issue_to_address>
### Comment 3
<location path="scripts/run_cross_sqlite_matrix.py" line_range="180" />
<code_context>
subprocess.run([binary, "--version"], capture_output=True, text=True, check=True)
</code_context>
<issue_to_address>
**security (python.lang.security.audit.dangerous-subprocess-use-audit):** Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.
*Source: opengrep*
</issue_to_address>
### Comment 4
<location path="scripts/run_cross_sqlite_matrix.py" line_range="183-185" />
<code_context>
completed = subprocess.run(
[binary, "-batch"], input=TIER_A_SQL, capture_output=True, text=True, check=True
)
</code_context>
<issue_to_address>
**security (python.lang.security.audit.dangerous-subprocess-use-audit):** Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.
*Source: opengrep*
</issue_to_address>Sourcery assessment
Needs a human reviewer. 4 findings to address first, and the change establishes a new projection-integrity and search-evidence contract, and widens the agent-control version boundary to accept 0.5.x runtimes. If either decision is wrong, reverting stops future use but cannot retract receipts or review outcomes already produced, although the impact is informational and does not mutate canonical records or grant execution authority.
Blocking findings: scripts/run_cross_sqlite_matrix.py:64, scripts/run_cross_sqlite_matrix.py:52, scripts/run_cross_sqlite_matrix.py:180, scripts/run_cross_sqlite_matrix.py:185
Code Review by Qodo
1.
|
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb08ece794
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d057986ec
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
PR review loop owner override:
This note accepts stale required-review risk for this exact PR head only. It does not request bot review, resolve findings, or merge the PR. |
Closes no issue automatically; this is a branch promotion from
devtomain.Promotion Evidence
joefeser/artifact-memoryorigin/dev@eb08ece794d65cc04cb4f2d42ab957307d9e0483codex/promote-dev-to-main-eb08ece794d6starts ateb08ece794d65cc04cb4f2d42ab957307d9e0483origin/main@153f2843acc91ed48f53c6a06f0feb333cc9db8fmerge_commitCommits To Promote
Validation / Handoff
agent-control pr-loop --repo joefeser/artifact-memory --pr <PR_NUMBER> --base main --require-codex-review --quiet --jsonPolicy Notes
Post-Merge Reconciliation
dev, open a separate source-based branch/PR that names each selected commit and its review evidence. Do not infereb08ece794d65cc04cb4f2d42ab957307d9e0483..HEAD, merge unrelatedmain-only work, or patch/force-pushdevduring this promotion.Summary by Sourcery
Promote the v0.1.3 candidate with integrity-gated projections and expanded, receipt-backed search capabilities.
New Features:
Bug Fixes:
Enhancements:
CI:
Documentation:
Tests:
Chores: