testnet: promote audited rootless candidate - #1750
Merged
Merged
Conversation
Adds a SPARQL scalability lint that scans PR-changed source for query
shapes behind past production incidents and fails the check when a PR
ADDS one:
R1 unscoped-all-var-scan whole-store ?s ?p ?o scans
R2 graph-var-scan all-var triple inside GRAPH ?var — the
#1597 listGraphs-storm shape
R3 offset-pagination O(offset)-per-page walks + torn reads
R4 bucket-graph-scan unbounded scans over growing graph
families (_shared_memory/_meta/data
graph/_catalog) — the #1609 shape
Deliberate exemptions keep the blessed idioms green: FILTER EXISTS
existence probes (the FIXED #1597 listGraphs form), plain ASK,
LIMIT-without-ORDER-BY, VALUES-bound graph vars, exact per-KA graph
reads, and CONSTRUCT/INSERT/DELETE output templates.
Ratchet semantics: findings are fingerprinted by (rule, normalized
query text); pre-existing debt is grandfathered as notices (current
baseline ~52 findings) and only NEW findings block. Provably bounded
queries can be acknowledged in code with
'sparql-scan-allow: <rule> -- <justification>', making every allowed
scan a reviewed, diffable decision.
The scanner is zero-dependency (no pnpm install in the job), extracts
SPARQL from TS template literals (regex-literal-aware tokenizer;
interpolations normalized), and self-tests against 20 fixtures —
including the exact #1597 bad/fixed pair — before every scan.
The workflow runs on every pull_request (no branch filter, so stacked
PRs whose base is a feature branch are covered) and on merge_group so
the merge queue cannot stall once the check is marked required.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review follow-ups on #1745: - Multiset ratchet: the baseline is now fingerprint -> count, and each head occurrence consumes one. Duplicating a grandfathered query in a changed file previously passed as pre-existing; it now blocks as one new finding. The diff-gate core is extracted as computeDiffFindings() and covered by an integration self-test on a throwaway git repo (duplicate blocks / reindent stays grandfathered / new shape blocks). - Keyword-terminated triples: SPARQL allows a triples block to run straight into FILTER/OPTIONAL/BIND/... with no dot; the walker now flushes the preceding triple at keyword boundaries and before group-opening headers, so '?s ?p ?o FILTER(...)' is caught in default and GRAPH ?var scopes. String-literal contents are dropped during analysis so quoted keywords cannot confuse detection. - Scoped LIMIT exemption: only a TOP-LEVEL 'LIMIT without ORDER BY' exempts R1/R2 (and R4); a subquery LIMIT no longer suppresses findings for unrelated outer scans. Fail-closed corollary pinned by fixture: a subquery-local LIMIT does not exempt its own group's scan (pragma if intentional). - Fixture suite grows 20 -> 27, all analyzer fixes pinned; self-test (fixtures + diff gate) still runs before every CI scan. - Replace a stray NUL byte in the fingerprint separator with ':' (NUL made the file grep/diff-hostile). Full-tree baseline stays at 52 findings (18 R1 / 9 R2 / 12 R3 / 13 R4). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Classic branch-protection API 404s on this repo (main is governed by ruleset 14325863); document the fetch-modify-PUT and the merge-before- flip ordering so the check cannot deadlock open PRs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review follow-up on #1745: replace the hand-written JS/TS lexer in the SPARQL scale lint with ts.createSourceFile + a template-literal walk. The lexer was the fragile layer (regex-vs-division disambiguation, nested templates, escape handling — an IRI-safety regex containing a quote desynced an early version); the parser boundary removes that whole class. typescript is already a root devDependency, so the workflow gains only 'pnpm install --frozen-lockfile --ignore-scripts' (~8s warm) and no new dependency. Extraction contract unchanged; template literals nested inside interpolations are now analyzed as their own nodes. Behavior verified identical: 27/27 fixtures + diff-gate self-test pass, full-tree audit unchanged at 52 findings (18 R1 / 9 R2 / 12 R3 / 13 R4), ~1s runtime. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
release: promote validated rootless KA candidate to main
mock-adapter-parity.test.ts [CH-8] enforces that MockChainAdapter mirror every EVMChainAdapter public method minus documented exemptions. Canary commit c4fd81e added verifyContractSignature (an EIP-1271 isValidSignature on-chain view call) to EVMChainAdapter, turning the audit red with missing = ['verifyContractSignature']. Resolved via the exemption route the test's own comment sanctions ('Either add it to the mock or put it in MOCK_EXEMPT_FROM_EVM with a comment'), rather than a mock impl: the method's only production caller (dkg-publisher.ts contract-author update authorization) is gated by hasContractCode(author), which the mock hardcodes false, so the branch is structurally unreachable on the mock. A true shim would be a false-accept on a security path; a false shim would be dead code; a faithful impl would have to model deployed 1271 bytecode. The two tests exercising the contract-author branch stub both methods per-test, so no mock-based flow needs a native impl. Same family as the on-chain-derived helpers already exempted. Verified: parity 15/15 (was red with exactly ['verifyContractSignature']); teeth intact — any other unmirrored EVM method still reddens the audit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tests Two GraphSetIndexStore tests construct a StorePriorityScheduler with a contrived maxConcurrent:2 pool. Canary commit 4057c25 added a health lane (DEFAULT_HEALTH_RESERVED_SLOTS=1); with the tests not setting healthReservedSlots, the health lane now reserves the 2nd of 2 slots and the normalReservedSlots:1 floor is clamped to 0, so the normal read can no longer bypass the in-flight background seed — turning both tests red ('lets a normal seed bypass an in-flight background seed', 'ignores an older failed background revalidation'). This is a stale test config, not a code regression: production uses DEFAULT_MAX_CONCURRENT=4, so after ack(1)+health(1) the ordinary pool is still 2 and normal+background run concurrently — the bypass invariant the tests guard still holds in prod. Pinning healthReservedSlots:0 restores the 2-slot ordinary pool the tests were designed around. No assertion is touched; every behavioral check (listGraphsCalls, options ordering, inflight snapshot counts, next-scan timing) is unchanged. Verified: 37/37 green; teeth intact — flipping healthReservedSlots back to 1 re-reds exactly these two tests; full storage suite 437 passing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tent KA
The rootless cutover moved the update authorization check
(assertValidPrecomputedUpdateAttestation) ahead of staging, where it reads
chain-truth via getKnowledgeAssetOwner -> ownerOf BEFORE the chain-submit
try/catch. For a non-existent or expired KA, ownerOf reverts
(ERC721NonexistentToken / KnowledgeAssetExpired), and update() propagated
that revert as a throw — breaking the fail-closed contract that update()
returns {status:'failed'} and mutates nothing when there is no updatable KA
on chain.
Wrap the pre-staging authorization in a try/catch that converts ONLY that
definitive 'no updatable KA' revert class to a failed result (mirroring the
existing V10_DEFINITIVE_ERRORS handling on the submit path). Every
authorization failure still throws: wrong owner of an existing KA
(KA_UPDATE_AUTHOR_NOT_OWNER), signer mismatch, and missing-adapter-method
config errors are re-raised unchanged, preserving reject-unauthorized-
before-staging.
Verified against real Hardhat: ka-update 20/20, security-regressions 26/26
(both were red on the non-existent-KA update). Teeth: mutating the
classifier to re-raise the nonexistent case re-reds the test, so the fix is
load-bearing. publisher-evm-e2e 'updating non-existent KC returns failed'
exercises the same path.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…r submit path Addresses PR #1744 review on the update() fail-closed fix: 1. Centralize the update-rejection taxonomy (review: two inline classifiers + two failed-result constructors in a 9k-line method). Extract extractV10UpdateRejectionName() and named PRE_STAGING_NO_UPDATABLE_KA_ERRORS / V10_DEFINITIVE_UPDATE_ERRORS constants, and a single buildFailedUpdateResult() that both the pre-staging owner check and the chain-submit path feed. No behavior change; the two error sets stay intentionally distinct (pre-staging is a strict subset — auth failures must still throw before staging). 2. Drop KnowledgeAssetExpired from the pre-staging set: it is thrown only by KnowledgeAssetsLifecycle at update SUBMIT, never by the storage contract's ownerOf (which the pre-staging check calls), so it was dead code there. 3. Cover the previously-untested submit-definitive path: new mock-based ka-update-submit-failure.test.ts drives update() past pre-staging and rejects the broadcast, proving KnowledgeAssetExpired / CannotUpdateImmutable map to {status:'failed'} with no store mutation, and a non-definitive rejection still throws (teeth). No Hardhat/epoch time-travel needed. Also corrects the stale 'V10 catches KnowledgeAssetExpired' comment in security-regressions.test.ts (that non-existent-KA path is now ERC721NonexistentToken at pre-staging). Verified: new test 3/3; publisher unit suite 417; ka-update 20/20 + security-regressions 26/26 unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review follow-ups on #1745 (round 3): - .tsx/.jsx files are now scanned, with ScriptKind derived from the extension (parsing .ts as TSX misreads generic arrows as JSX and vice versa — both can silently drop template literals from the AST). The concern was live, not hypothetical: the expanded audit surfaces a real R2 graph-var scan in node-ui's project query.tsx component (grandfathered; baseline 52 -> 53). Diff-gate self-test now includes a changed .tsx file whose JSX only parses under TSX, carrying an R2 shape that must block; a generic-arrow-in-.ts fixture pins the ScriptKind split. - --all and --files now run the embedded self-test before scanning, matching --diff: audit FINDINGS stay non-blocking, but a broken scanner fails the run instead of reporting garbage with exit 0. Proven by a spawned-CLI check inside the diff-gate self-test (env-guarded against recursion) that requires the self-test banner before the findings output. - Capture git stderr in fileAt(): probing base versions of added files is expected to fail and must not print 'fatal:' noise into CI logs. 28/28 fixtures + diff-gate pass; full-tree audit 53 findings (18 R1 / 10 R2 / 12 R3 / 13 R4) across 1101 scanned files, ~1s. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…pdate-main fix(publisher): classify rejected KA updates
ci: restore independent Solidity path gating
ci: block PRs that add O(store) SPARQL query shapes
Authenticate durable sync controls and advance safely past fully discarded metadata-only pages.
…dentity-preflight
…preflight Reject graph-scoped publish identity conflicts before mint and canonicalize numeric EVM chain aliases.
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
3 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.
Promotes exact main at f4c4efa to testnet-canary for the private-CG rootless KA certification run. Includes the SPARQL scale gate (#1745), authenticated durable sync controls (#1748), and graph-scoped pre-mint identity/chain validation (#1749).