fix(bcos): reject duplicate cert_id instead of silently overwriting + erasing anchor#8011
Open
Vyacheslav-Tomashevskiy wants to merge 1 commit into
Conversation
… erasing anchor bcos_attest stored with INSERT OR REPLACE, but cert_id is UNIQUE and the handler carries an `except sqlite3.IntegrityError -> 409 "already exists"` guard. INSERT OR REPLACE resolves a UNIQUE conflict with a silent DELETE+ INSERT and never raises IntegrityError, so the 409 guard was dead code: a second attest for a known cert_id silently overwrote repo/tier/trust_score/ reviewer/report. The REPLACE column list also omits anchor_tx and passes anchored_epoch=None, so re-attesting a ledger-anchored cert wiped its anchor_tx back to NULL. That breaks /api/v1/bcos/anchor's documented idempotency (an already-anchored cert should return its existing tx hash, not write a second ledger row): after the wipe a re-anchor writes a second ledger row with a fresh tx hash. Because auth passes on admin key OR a valid Ed25519 signature over a caller-supplied signer_pubkey, any party could self-sign a report and overwrite a public cert_id. Fix: plain INSERT so a duplicate raises IntegrityError and the existing 409 guard fires, preserving the original record and its anchor. Same overwrite class as node/tests/test_attestation_overwrite_reward_loss.py. Adds node/tests/test_bcos_reattest_preserves_anchor.py (fails on old code, passes on fix; existing 8 bcos tests still green). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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
bcos_attest()(node/bcos_routes.py) stored the certificate withINSERT OR REPLACE, butcert_idisUNIQUE(schema line 184) and the handler carries anexcept sqlite3.IntegrityError -> 409 "Certificate ... already exists"guard.INSERT OR REPLACEresolves a UNIQUE conflict with a silent DELETE + INSERT, so it never raisesIntegrityError. The 409 guard was therefore dead code, and a second attest for a knowncert_idsilently overwroterepo/tier/trust_score/reviewer/report.The REPLACE column list also omits
anchor_txand passesanchored_epoch=None, so re-attesting a ledger-anchored certificate wiped itsanchor_txback toNULL. That breaks the documented idempotency of/api/v1/bcos/anchor(an already-anchored cert should return its existing tx hash, not write a second ledger row): after the wipe, a re-anchor writes a second ledger row with a fresh tx hash for the same certificate.Auth passes on
is_admin OR valid Ed25519 signatureover a caller-suppliedsigner_pubkey, so any party can self-sign a report they authored and overwrite a known publiccert_id.Repro (deterministic)
BCOS-x(repoScottcjn/Rustchain, L2, trust 90) -> anchor it (anchor_txset).cert_idwithrepo attacker/evil, trust 100.IntegrityError; row becomes{repo: attacker/evil, trust_score: 100, anchored_epoch: None, anchor_tx: None}-- anchor erased, trust forged, 409 never fires.Fix
Plain
INSERT(one word). The duplicate now raisesIntegrityError, the existing 409 guard fires, and the original record + its anchor are preserved.cert_idis meant to be immutable once issued -- exactly what the "already exists" 409 already intended.Same overwrite class the repo already treats as a real bug in
node/tests/test_attestation_overwrite_reward_loss.py(that one isminer_attest_recent; this is the untouchedbcos_attestationstable).Tests
node/tests/test_bcos_reattest_preserves_anchor.py-- fails on old code, passes on the fix (mutation-checked).node/tests/test_bcos_routes.py(8 tests) still green.RTC reward address:
RTCd1554f0f35576faf01d386a6be1c947f560dd0b7