[smith-xyz]

Proven bugs (red → green)

where	why it is wrong	red test	state
ledger.py verify_merkle_integrity	bare int(meta.get("merkle_epoch", 0)) — a tampered layer with a non-integer epoch raised TypeError/ValueError out of the tamper-evidence reader instead of returning an IntegrityFinding (leaf_format 30 lines below already did the defensive coercion)	test_non_numeric_epoch_is_error[None|x] — ValueError at ledger.py:329	fixed
ledger.py verify_merkle_signature	identity-method dispatch reported an intact layer as "verification failed" (ERROR, tamper-evidence language) for a config gap, while the keypair analogue is a WARNING	test_..._identity_without_policy_reports_unconfigured_not_failed	fixed, then superseded — see the removal below
ledger.py merkle_root_payload	format-1 path (the default when merkle_signature_format is absent) fed untrusted merkle_root into bytes.fromhex → ValueError escaped the verifier uncaught instead of an IntegrityFinding	test_..._non_hex_root_is_finding_not_crash — ValueError at ledger.py:478	fixed

Review round 1 (Qodo @ 98d1b97 + end-to-end review) — commit 6a5a3ee

Every guard widened below was red on 98d1b97 before the fix (bug-sniff discipline: proof before code); Qodo's re-review of 6a5a3ee auto-attributed all three as implemented.

finding	why it is a real defect	red test on 98d1b97	fix
Qodo 3d70d784: non-string root still crashes	bytes.fromhex(5) raises TypeError, not ValueError — the format-1 guard caught only ValueError, so merkle_root=5 escaped the verifier	test_..._non_hex_root_is_finding_not_crash[5] — TypeError at ledger.py:444	guard catches (TypeError, ValueError); test parametrized with 5
Qodo 749a687c: infinite epoch still crashes	int(float("inf")) raises OverflowError — neither the epoch guard nor the leaf_format coercion caught it (the leaf-format gap was the same class, unflagged by both reviews)	test_non_numeric_epoch_is_error[inf] and test_unknown_leaf_format_is_error[inf] — OverflowError	both coercions catch OverflowError
Qodo 96801ed3: AGENTS.md listed the removed backend	agents/maintainers reading AGENTS.md would expect keyless identity signing that no longer exists	—	AGENTS.md + a stale (OIDC) SignResult comment updated

Open — Qodo 2bcc5d78 (action_required), parked on a data check, deliberately not softened:
Removing the identity backend makes a layer marked merkle_signing_method='identity' get ERROR "not a recognized signing method", which blocks append/seal on such rows in core. Justification for keeping the ERROR: post-removal, no code exists that can verify an identity signature — a caller "with the identity policy" has nothing to consume it — so the only question is whether historical identity rows exist. Core has only ever built SigningConfig(method="keypair") (its LedgerService._signing), and the 2026-08-17 incident's signed layers were keypair, so the expectation is zero rows — but that is data, not code, so it stays open. Action: run SELECT merkle_signing_method, count(*) FROM traust_ledger.layers GROUP BY 1 on production. If only keypair → dismiss this finding and drop identity from traust-contracts' signing-method enum (follow-up, XWING-2363). If identity rows exist → restore verification for them via a migration; a WARNING verdict is rejected because under required=True it would open a method-flip laundering path (flip the method, skip signature verification, get re-signed).

History note: commit 32f9146 (the identity-dispatch WARNING fix) is superseded by the dead-code removal — kept rather than squashed so each red-test proof stays reviewable; Qodo re-reviews each new head regardless.

Dead code removed (service-era surface core never wires)

traust-core — the only consumer — imports 11 names; everything else was loaded from the deleted service's LAAS_SIGNING_* env config and kept alive by the library-only refactor (7f34ff6):

removed	why it is dead
SigstoreOIDCBackend + oidc_* / allow_interactive / ca_url / tlog_url / interactive OAuth2	identity verification was never wired in core (SigningConfig(method="keypair") only); custom-URL trust config was env-service config, built by f-string JSON interpolation
stamp_and_sign	exported as "the single entry point every write path should use", but core hand-rolls the sequence in _sign with the correct staleness check (merkle_root_signature is not None) — stamp_and_sign's root-moved check fabricated a drop alarm on never-signed re-writes
rekor opt-in on sign	contradicted the policy of record (transparency-log publication forbidden); the never-publish flags and post-condition discard are now unconditional
dead references	format-1 warning pointed at sign_merkle_root.py (deleted in 7f34ff6); SignAttempt.warning() docstring's "six call sites" (one caller remains)

signing.py 719 → 418 lines. Kept: the RFC 6962 proof surface and restatements module — audit-facing primitives, flagged in status.md for a product call.

Needs your verification (data behaviour)

where	red test	what to check
identity-marked layers (Qodo 2bcc5d78)	—	SELECT merkle_signing_method, count(*) FROM traust_ledger.layers GROUP BY 1 on production; see the justification above

Coverage

63/63 units (100%) judged across src/traust_ledger/integrity in one wave: 60 clean, 3 proven → fixed here. Ledger: progress-tracker tracking/bug-campaign/traust-ledger.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: smith-xyz

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [smith-xyz]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[qodo-for-traust-security]

PR Summary by Qodo

Harden Merkle verification and simplify offline keypair signing

🐞 Bug fix ✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Return integrity findings instead of crashing on malformed epochs and legacy signature roots.
• Remove unused identity signing and signing shortcuts, leaving an offline keypair path.
• Update tests and documentation for explicit stamping and signing without transparency-log
 publication.

Diagram

graph TD
  Writer["Layer writer"] --> Stamp["Merkle stamping"] --> Layer["Layer metadata"] --> Sign["Keypair signing"] --> Cosign["Offline cosign"]
  Layer --> Integrity["Integrity verifier"] --> Findings["Integrity findings"]
  Layer --> Signature["Signature verifier"] --> Findings
  Cosign -->|signature| Layer

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Isolate legacy identity verification

• ➕ Could preserve verification of any historical identity-signed layers without retaining OIDC signing in the core package.
• ➖ Requires an identity policy, external trust infrastructure, and a maintained integration for an unwired consumer path.

Recommendation: Keep the keypair-only path if the consumer inventory confirms identity signing was never used. Before treating historical signature support as unchanged, establish whether identity-signed artifacts exist; if they do, consider a separately configured verification adapter.

Files changed (12) +144 / -594

Bug fix (1) +41 / -51

ledger.pyReport malformed Merkle metadata as findings +41/-51

Report malformed Merkle metadata as findings

• Converts non-integer epochs and non-hex roots in legacy signature payloads into error findings. Removes the unused combined signing helper and the Rekor option from configured signing.

src/traust_ledger/integrity/ledger.py

Refactor (2) +21 / -315

__init__.pyRemove obsolete signing exports +0/-4

Remove obsolete signing exports

• Stops exporting the removed combined signing helper and OIDC backend.

src/traust_ledger/integrity/init.py

signing.pyRestrict signing to offline cosign keypairs +21/-311

Restrict signing to offline cosign keypairs

• Removes OIDC signing, its configuration, and backend dispatch. Eliminates the Rekor opt-in so cosign always disables transparency-log upload and rejects bundles containing publication evidence.

src/traust_ledger/integrity/signing.py

Tests (7) +72 / -222

test_cosign_v3.pyRemove the Rekor opt-in test +0/-30

Remove the Rekor opt-in test

• Deletes the test that permitted transparency-log publication, which is no longer supported.

tests/test_cosign_v3.py

test_e2e_merkle_pipeline.pyAlign pipeline stub with the signing interface +1/-1

Align pipeline stub with the signing interface

• Removes the obsolete Rekor argument from the end-to-end test backend.

tests/test_e2e_merkle_pipeline.py

test_layer_verification.pyTest malformed legacy roots without cosign +46/-3

Test malformed legacy roots without cosign

• Adds a stub-backed regression test asserting that a non-hex format-1 root yields an error finding. Updates signed-chain tests to stamp and sign explicitly.

tests/test_layer_verification.py

test_provider_logging.pyRetain logging coverage for the supported provider +4/-156

Retain logging coverage for the supported provider

• Removes OIDC-specific logging tests and uses cosign to retain coverage that logging failures do not replace provider exceptions.

tests/test_provider_logging.py

test_sign_if_configured.pyExercise stamping and signing separately +9/-27

Exercise stamping and signing separately

• Replaces combined-helper calls with explicit stamping and configured signing. Removes tests for the obsolete OIDC and combined-helper paths.

tests/test_sign_if_configured.py

test_signing.pyAssert cosign always disables log upload +4/-5

Assert cosign always disables log upload

• Removes Rekor-enabled signing calls and changes command assertions to require the transparency-log disabling flag.

tests/test_signing.py

test_verify_merkle_integrity_edges.pyCover invalid Merkle epoch types +8/-0

Cover invalid Merkle epoch types

• Adds parameterized cases confirming that null and nonnumeric epochs return error findings.

tests/test_verify_merkle_integrity_edges.py

Documentation (2) +10 / -6

README.mdShow the supported two-step signing flow +9/-3

Show the supported two-step signing flow

• Replaces the removed stamping-and-signing helper with explicit stamping followed by configured signing. Describes private-key cosign signing as offline.

README.md

ledger-integrity.mdDocument keypair-only signing +1/-3

Document keypair-only signing

• Removes references to the combined helper and identity-token signing from the ledger integrity guide.

docs/ledger-integrity.md

[qodo-for-traust-security]

Code Review by Qodo

1. Historical identity signatures cannot verify 🐞 Bug

Description

Removing the identity backend from _METHOD_BACKENDS makes get_backend_for_method("identity")
reject that recorded signing method. A historical layer carrying an identity signature now receives
an error finding instead of reaching signature verification, even when the caller has the identity
policy needed to verify it.

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Historical layers declaring the identity signing method lose their verification path when its backend is removed.
## Fix Focus Areas
- src/traust_ledger/integrity/signing.py[381-394]
- src/traust_ledger/integrity/ledger.py[548-573]
## Recommended Fix
Retain a verification-only identity backend and a way for callers to supply its trusted identity policy, without restoring identity signing or transparency-log publication. Add regression coverage for an identity-signed historical layer.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📜 Skill insights (0)

1. Historical identity signatures stop verifying 🐞 Bug ≡ Correctness

Description

get_backend_for_method() no longer recognizes the stored identity signing method because its
backend registration was removed. When a caller verifies an identity-signed historical layer with a
public key configured, verify_merkle_signature() returns an unknown-method error rather than
checking its existing signature.

Code

src/traust_ledger/integrity/signing.py[660]

-    "identity": SigstoreOIDCBackend,

Evidence

The historical-layer contract says older signed artifacts remain verifiable; the verifier dispatches
on their persisted method, but the changed registry has no identity entry. The removed identity
backend and tests establish that this method previously had a verification implementation.

docs/ledger-integrity.md[57-61]
src/traust_ledger/integrity/ledger.py[566-579]
src/traust_ledger/integrity/signing.py[382-414]
AGENTS.md[10-24]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Removing the identity backend registration makes previously supported identity-signed historical layers unverifiable.

## Fix Focus Areas
- src/traust_ledger/integrity/signing.py[382-414]
- src/traust_ledger/integrity/ledger.py[566-579]
- docs/ledger-integrity.md[57-61]

## Recommended Fix
Retain a verification-only identity backend and method registration with explicit identity-policy configuration, even if new identity signing is retired. Add a regression test that verifies a historical identity-signed layer through the public verifier.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗

---

2. A non-string legacy root still crashes the verifier ✓ Resolved

Description

In the format-1 branch, verify_merkle_signature only catches ValueError around
merkle_root_payload(merkle_root). If a tampered layer has a truthy non-string merkle_root (for
example 123, a list or a dict) and no merkle_signature_format, bytes.fromhex raises
TypeError, which escapes to the caller. That is the same crash this fix is meant to close, just
reached through a different malformed field type.

Code

src/traust_ledger/integrity/ledger.py[R614-616]

+        try:
+            payload = merkle_root_payload(merkle_root)
+        except ValueError:

Evidence

The only guard before the call is the truthiness check if not merkle_root, so 123 gets through.
merkle_root_payload passes the value straight to bytes.fromhex, which raises TypeError for
anything that isn't a str. The new handler names only ValueError. By contrast, the epoch fix in the
same PR catches (TypeError, ValueError).

src/traust_ledger/integrity/ledger.py[437-444]
src/traust_ledger/integrity/ledger.py[561-568]
src/traust_ledger/integrity/ledger.py[613-626]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The format-1 path in `verify_merkle_signature` catches only ValueError from `merkle_root_payload`. A non-str `merkle_root` raises TypeError from `bytes.fromhex`, and that exception escapes the verifier.

## Fix Focus Areas
- src/traust_ledger/integrity/ledger.py[614-626]
- tests/test_layer_verification.py[77-96]

## Recommended Fix
Change the handler to `except (TypeError, ValueError):`. Add a parametrized regression test that covers `merkle_root` values `123` and `["ab"]` in addition to the existing non-hex string.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Writers lose the unsigned-layer warning 🐞 Bug ◔ Observability

Description

The replacement README sequence discards stamp_merkle_metadata()'s signature-dropped result, and
sign_if_configured() cannot carry it into SignAttempt.stale_signature_dropped. If events change
on a signed layer while no key is configured, stamping removes its signature but the resulting
attempt is unconfigured with no warning.

Code

README.md[R18-19]

+stamp_merkle_metadata(layer)  # recompute root; a changed root drops a stale signature
+attempt = sign_if_configured(layer, SigningConfig(method="keypair", key_path="cosign.key"))

Evidence

Stamping explicitly returns whether it dropped a stale signature, while the replacement sequence
ignores that return. The unconfigured signing path creates an attempt without the dropped flag, so
its warning method returns None; the deleted combined helper had propagated the flag.

README.md[18-19]
src/traust_ledger/integrity/ledger.py[55-61]
src/traust_ledger/integrity/ledger.py[109-120]
src/traust_ledger/integrity/ledger.py[149-178]
src/traust_ledger/integrity/ledger.py[180-200]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The split writer sequence loses the signal that a changed root dropped an existing signature when re-signing is unconfigured.

## Fix Focus Areas
- README.md[18-19]
- src/traust_ledger/integrity/ledger.py[149-178]
- src/traust_ledger/integrity/ledger.py[180-224]
- tests/test_sign_if_configured.py[118-148]

## Recommended Fix
Provide a writer-facing way to combine the stamp result with the signing attempt, and surface a warning when a signature was dropped without replacement. Update the example to use it and restore regression coverage for that transition.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗

---

4. An unsupported signing method passes silently as unconfigured 🐞 Bug ◔ Observability

Description

sign_if_configured now checks only key_path. A SigningConfig(method="identity") or any other
unsupported method with no key_path returns SignAttempt("unconfigured"), and
SignAttempt.warning() treats that as nothing to report. A caller whose config still says identity
therefore never signs and gets no warning, while the same config with a key path fails loudly.

Code

src/traust_ledger/integrity/ledger.py[R197-199]

    key_path = getattr(config, "key_path", None)
-
-    # Only the keypair backend needs a key path; identity signing derives an
-    # ephemeral key from an OIDC token, so treat a missing token as unconfigured
-    # too rather than attempting an interactive flow inside a batch writer.
-    if method == "keypair":
-        if not key_path:
-            return SignAttempt("unconfigured")
-    elif not getattr(config, "oidc_token", None):
-        return SignAttempt("unconfigured", method=method)
+    if not key_path:
+        return SignAttempt("unconfigured")

Evidence

The method check happens only inside get_backend_from_config, which runs after the key_path
early return. The docstring describes unconfigured as the normal state and says it needs no
output.

src/traust_ledger/integrity/ledger.py[180-213]
src/traust_ledger/integrity/signing.py[404-414]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`sign_if_configured` returns `unconfigured` for any method when `key_path` is unset, so a config with a retired or unknown method never surfaces.

## Fix Focus Areas
- src/traust_ledger/integrity/ledger.py[197-199]

## Recommended Fix
Before the `key_path` check, return `SignAttempt("failed", method=config.method, error=...)` when `config.method` is not in the supported methods (`keypair`). Add a test for it.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗

---

5. An infinite epoch value still crashes the integrity check ✓ Resolved

Description

The new guard around int(meta.get("merkle_epoch", 0)) catches TypeError and ValueError but not
OverflowError, which is what int(float('inf')) raises. Python's default json.loads turns the
token Infinity into float('inf'), so a tampered layer document with "merkle_epoch": Infinity
still raises an exception out of verify_merkle_integrity instead of returning a finding.

Code

src/traust_ledger/integrity/ledger.py[R295-297]

+    try:
+        epoch = int(meta.get("merkle_epoch", 0))
+    except (TypeError, ValueError):

Evidence

int(float('inf')) raises OverflowError (int(float('nan')) raises ValueError and is caught). Only
(TypeError, ValueError) are handled at line 297. Whether this can be reached depends on how layers
are deserialized: the stdlib json parser accepts Infinity, while some stores (for example JSONB)
reject it. This is an inspection finding, not a reproduction.

src/traust_ledger/integrity/ledger.py[295-307]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`verify_merkle_integrity` coerces `merkle_epoch` with `int()` and catches only TypeError/ValueError. An infinite float raises OverflowError, which escapes the verifier.

## Fix Focus Areas
- src/traust_ledger/integrity/ledger.py[295-307]
- tests/test_verify_merkle_integrity_edges.py[99-106]

## Recommended Fix
Use `except (TypeError, ValueError, OverflowError):` and add `float('inf')` to the parametrized `test_non_numeric_epoch_is_error` cases.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

Review mode: Auto: 🧠 Deep: Security-sensitive signing changes span multiple independent integrity and API paths.

Tip of the day

💡 Did you know, you can copy the agent prompt from any finding and feed it to your IDE agent

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[smith-xyz]

Review round 1 — dispositions (commit 6a5a3ee)

Each fix was red on 98d1b97 before the change; Qodo's re-review of 6a5a3ee auto-attributed the three as implemented.

Qodo finding	verdict	what was done
3d70d784 non-string root crash	accepted, fixed	bytes.fromhex(5) raises TypeError, which the ValueError-only guard missed → guard catches (TypeError, ValueError); test parametrized with 5 (red: TypeError at ledger.py:444)
749a687c infinite epoch crash	accepted, fixed	int(float("inf")) raises OverflowError → epoch guard catches it; the leaf_format coercion had the same unflagged gap and got the same fix (both red first)
96801ed3 AGENTS.md lists removed backend	accepted, fixed	AGENTS.md + a stale (OIDC) comment no longer advertise identity signing
2bcc5d78 historical identity signatures error	open by design, parked on data	No code can verify an identity signature post-removal, so a caller-side policy is moot; the question is only whether such rows exist. Core has only ever wired keypair. Needed: SELECT merkle_signing_method, count(*) FROM traust_ledger.layers GROUP BY 1 on production. Only-keypair → dismiss + drop identity from traust-contracts (XWING-2363). Rows exist → migration, not a softer verdict: a WARNING under required=True would open a method-flip laundering path.

History: 32f9146 (identity-dispatch WARNING) is superseded by the dead-code removal and kept for its red-test proof rather than squashed.

[smith-xyz]

I'm okay with us removing the oidc for now it was never used but I wish it was :)