feat(signing): register NEAR/WC providers + flip production to durable composition (attested-signing PR13)

[zmanian]

Revival cascade — re-ported onto current main (2026-07-23)

This branch was force-pushed with a fresh port of PR13 onto current main (the dormant stack was 1184 commits behind). The base (attested-signing-12-durable-stores) is likewise the re-ported PR12, so the stack is internally consistent. PR13's own delta is the same three commits (register NEAR/WC providers + the henrypark133 paranoid-architect review + state_secret entropy-on-decoded-hex), plus one cascade cleanup commit.

What changed vs the pre-port branch (drift reconciliation against main):

• libsql version skew fixed. ironclaw_attested_store was pinned at libsql 0.6; the workspace standardized on 0.9 (filesystem/host_runtime). The durable-assembly module (composition, libsql 0.9) was handing a 0.9 Database to the store's 0.6 stores → E0308 "multiple libsql versions". Bumped the store to 0.9 (API-compatible, no drift); also dropped a now-dead GateRef import in ledger.rs (the PR12 LedgerKey tenant-scoping made it unused).
• Composition gained additive postgres / libsql forwarding features (ironclaw_attested_store/{postgres,libsql}). Current-main composition is backend-agnostic; the attested_durable module is fully #![cfg(all(attested-broadcast, any(libsql, postgres)))] and inert until a deployment opts in. deadpool-postgres/libsql are already unconditional deps, so no dep: toggle was needed.
• henrypark133 review fixes preserved on top of the new base: downcast-failure now maps to BackendUnavailable (503, a composition-wiring bug must not masquerade as a 400 client proof rejection); ContinuationError::Config → BackendUnavailable; the IDOR MissingBinding → 404 mapping (from PR11) is retained (_scope/_run_id were not renamed to unused — this branch's PR11 threads scope/actor for the owner-scoped IDOR check).

Verification (this port, run with IRONCLAW_DISABLE_OS_KEYCHAIN=1):

• cargo fmt + cargo clippy --tests (default and libsql/postgres/attested-broadcast features) — zero warnings.
• Attested suites green: ingress 8/8 (incl. cross-user IDOR + forged-signature), provider-registration 9/9 (incl. NEAR partial-config fail-closed), durable-assembly 4/4 (libsql SSRF metadata-URL reject + reassembly persistence).
• workspace_graph_is_openssl_free green (the libsql 0.9 bump keeps the rustls-only TLS stack); threat-matrix 21/21; ironclaw_product_workflow 420 + 97 lib.

---

Final wiring PR (PR13/13) of the attested-signing substrate. Closes the two gaps PR11 and PR12 deferred.

Merge note for reviewers

This branch merges #3995 (PR11, reborn webui ingress + provider registry) into the PR12 (durable stores) base — both descend from PR10 and both edit the composition crate. The first commit is the merge resolution; reviewers should focus on the commits after the merge (the feat(signing): register NEAR/WC providers ... commit).

Merge resolution (attested.rs / runtime.rs / attested_continuation.rs)

UNION of both sides:

• attested.rs: kept PR12's generic RebornAttestedComposition<B,G,L> + durable type aliases AND PR11's grants field, register_attested_gate, and bindings() accessor (now generic).
• runtime.rs build_attested_composition: kept PR11's injected-provider registration AND PR12's new_in_memory constructor.
• attested_continuation.rs / the PR11 ingress test: made generic over <B,G,L> to match PR12's struct; coerced the resume-port binding to Arc<dyn SyncBindingRead> (PR12 signature change).

Gap 1 — register NEAR + WalletConnect providers

New AttestedProvidersConfig (composition layer, attested_config.rs):

• Injected always registered.
• NEAR redirect registered only when wallet base URL + callback URL + state_secret are all present. state_secret is a secret: SecretString, redacted Debug, env-only (never the operator TOML), mirroring the CUSTODIAL_MAINNET_ENABLED convention.
• WalletConnect registered only when the publishable ProjectId is present (shareable app identity, not a per-tenant secret). Uses the in-memory SessionBindingStore the provider builds internally (PR12 added no durable session-binding store).
• Each provider is independently fail-closed: missing config → unregistered → ProviderMismatch. No placeholder secrets.

Gap 2 — durable composition seam

New attested_durable.rs (feature-gated on attested-broadcast + libsql/postgres): assemble_libsql / assemble_postgres build the durable *AttestedComposition from a DB handle, ChainRpcEndpoints, and AttestedProvidersConfig. Backend choice follows the configured DB backend (mirrors every other reborn store). RPC endpoints and providers are independently fail-closed.

Scope note: the production runtime entrypoint is still local-dev-gated (build_reborn_runtime rejects non-local-dev; the CLI bails before reaching it), and build_*_production returns substrate RebornServices without assembling a RebornRuntime. So rather than threading config through a path that does not connect yet, PR13 lands the config-explicit, dual-backend-tested assembly seam the production-runtime slice will call. The runtime-field erasure + production-runtime wiring is documented as a deferred follow-up in attested_durable.rs.

Tests (through the caller)

• attested_provider_registration.rs: NEAR + WC unconfigured → ProviderMismatch; configured → reach verify_resume (not ProviderMismatch), driven through the assembled composition's driver().
• attested_durable_assembly.rs: durable libSQL composition assembles via assemble_libsql, runs migrations, registers the configured provider, and drives the durable driver.

Verification

• cargo fmt --all
• cargo clippy --all --tests --examples --all-features -- -D warnings — exit 0
• cargo test -p ironclaw_reborn_composition -p ironclaw_attested_runtime -p ironclaw_attested_store -p ironclaw_wallet_external -p ironclaw_architecture — green
• libsql-only and postgres-only builds compile (± attested-broadcast)
• cargo tree -i openssl-sys (host + x86_64-unknown-linux-gnu) — empty; workspace_graph_is_openssl_free green

(The pre-port note about 2 pre-existing facade_factory.rs --all-features failures no longer applies — those were fixed on main before this port.)

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request implements the crypto-free attested-signing continuation port for the WebUI facade. It introduces the AttestedGateContinuationPort trait and the AttestedProofClaim DTO to allow the product-facing facade to drive deterministic sign and broadcast operations without directly handling cryptographic types. The implementation includes environment-based configuration for NEAR and WalletConnect providers, durable assembly logic for PostgreSQL and libSQL backends, and comprehensive end-to-end tests for the resolution ingress. Feedback was provided regarding a potential panic in the manual hex parsing logic; specifically, slicing a string without verifying ASCII boundaries can cause a crash if non-ASCII characters are present, so an explicit validation check was suggested.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6e47d47de1

ℹ️ 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".

[chatgpt-codex-connector]

Accept legacy signer key in injected-wallet proof payload

The new injected-wallet decoder only deserializes claimed_signer, but the existing gate-resolve wire contract still uses signer (see src/channels/web/types.rs and src/channels/web/features/chat/attested.rs, where input.signer is mapped into claimed_signer). With this change, payloads that follow the established shape will deserialize as malformed and attested gate resolution will fail even when the signature is valid. Add a serde alias (or rename) so both keys are accepted for compatibility.

Useful? React with 👍 / 👎.

[zmanian]

Good catch — fixed in 0eb827d. Added #[serde(alias = "signer")] to InjectedWalletProofInput::claimed_signer so the durable v2 ingress decodes the established browser wire shape (signer, per src/channels/web/types.rs and the legacy attested.rs path) while keeping the unambiguous claimed_signer as canonical. Added injected_decoder_accepts_legacy_signer_alias + ..._canonical_claimed_signer tests. WalletConnect has no prior signer wire contract, so I left its canonical field unaliased.

[zmanian]

The wire contract this references (src/channels/web/types.rs and src/channels/web/features/chat/attested.rs, where input.signer was mapped into claimed_signer) was deleted in the v1 src/ monolith excision — this branch is re-ported onto post-deletion main. The Reborn attested-proof path deserializes claimed_signer directly from the WebUI attested_proof JSON with no v1 translation layer, and there is no current client sending the legacy signer shape (the injected-wallet browser flow is part of the still-to-land frontend work). Leaving this open as a reminder to pin the exact field name when the Reborn injected-wallet frontend lands — at which point a #[serde(alias = "signer")] is a cheap backward-compat option if we want to accept the legacy key.

[henrypark133]

Code Review (multi-agent)

Intent: Register NEAR/WalletConnect providers with fail-closed config and flip production to durable composition
Stats: 23 findings (from 29 raw) across 11 files. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, pattern-refactor. Reviewers failed: none. Body-only: 8

Security (1)

1. Medium state_secret entropy threshold too permissive (distinct < 8 bytes) (crates/ironclaw_reborn_composition/src/attested_config.rs:202-214, confidence 50) — anchor: crates/ironclaw_reborn_composition/src/attested_config.rs:214
   A 32+ byte key using only 8 distinct bytes still passes.
   Fix: Raise the distinct-byte threshold to at least 16.

Bugs (3)

1. High is_internal_host misses IPv6 link-local addresses (fe80::/10) (crates/ironclaw_reborn_composition/src/attested_durable.rs:143-153, confidence 100) — anchor: crates/ironclaw_reborn_composition/src/attested_durable.rs:151
   The is_internal_host function rejects IPv4 link-local but the IPv6 branch omits is_unicast_link_local(). fe80::1 and other fe80::/10 addresses pass through.
   Fix: Add || v6.is_unicast_link_local() to the IPv6 match arm.
   Also flagged by: tests/High
   Also flagged by: security/Medium

2. Medium ValidatedUrl Debug impl exposes raw URL which may contain sensitive query params (crates/ironclaw_reborn_composition/src/attested_config.rs:155-159, confidence 75) — anchor: crates/ironclaw_reborn_composition/src/attested_config.rs:156
   ValidatedUrl's Debug renders the full URL string verbatim.
   Fix: Change ValidatedUrl Debug to redact query and fragment portions.
   Also flagged by: local-patterns/Low

3. Medium parse_hex silently returns empty vec for empty or 0x-only input (crates/ironclaw_reborn_composition/src/attested_continuation.rs:204-216, confidence 75) — anchor: crates/ironclaw_reborn_composition/src/attested_continuation.rs:205
   parse_hex('') and parse_hex('0x') both return Ok(vec![]) instead of MalformedProof.
   Fix: Add an early guard for empty input after stripping the prefix.
   Also flagged by: tests/Medium
   Also flagged by: tests/Medium

Tests (7)

1. Medium Partial NEAR config (some env vars set, others absent) not tested (crates/ironclaw_reborn_composition/src/attested_config.rs:337-355, confidence 75) — anchor: crates/ironclaw_reborn_composition/src/attested_config.rs:337
   No test exercises the three partial-config permutations for NEAR.
   Fix: tests::attested_config::partial_near_config_fails_closed

2. High map_continuation_error tested for only 1 of 7 ContinuationError variants (crates/ironclaw_reborn_composition/src/attested_continuation.rs:278-310, confidence 100) — anchor: crates/ironclaw_reborn_composition/src/attested_continuation.rs:278
   Missing: MissingBinding, ProviderMismatch, ProofRejected(GrantClaimFailed), ProofRejected(other), Ledger, ChainSigning.
   Fix: tests::attested_continuation::map_continuation_error_all_variants

3. Medium NEAR redirect proof decode path has no dedicated test (crates/ironclaw_reborn_composition/src/attested_continuation.rs:124-155, confidence 75) — anchor: crates/ironclaw_reborn_composition/src/attested_continuation.rs:138
   No test exercises the NEAR redirect decode path including FunctionCall variant.
   Fix: tests::attested_continuation::near_redirect_proof_decode_covers_full_access_and_function_call

4. Medium WalletConnect proof decode path has no dedicated test (crates/ironclaw_reborn_composition/src/attested_continuation.rs:157-172, confidence 75) — anchor: crates/ironclaw_reborn_composition/src/attested_continuation.rs:157
   No test exercises the WalletConnect decode path or optional public_key handling.
   Fix: tests::attested_continuation::walletconnect_proof_decode_covers_with_and_without_public_key

5. High map_attested_continuation_rejection has no tests for any rejection variant (crates/ironclaw_product_workflow/src/reborn_services.rs:1295-1353, confidence 100) — anchor: crates/ironclaw_product_workflow/src/reborn_services.rs:1307
   The error-mapping function that translates all 7 AttestedContinuationRejection variants into HTTP status codes is completely untested.
   Fix: tests::reborn_services::map_attested_continuation_rejection_all_variants

6. Medium build_attested_composition with invalid provider config not tested (crates/ironclaw_reborn_composition/src/runtime.rs:924-960, confidence 75) — anchor: crates/ironclaw_reborn_composition/src/runtime.rs:938
   No integration test verifies that invalid env config causes build to fail.
   Fix: tests::runtime::build_reborn_runtime_fails_on_invalid_attested_provider_config

7. Medium register_attested_gate not tested through the assembled composition caller (crates/ironclaw_reborn_composition/src/attested.rs:135-165, confidence 75) — anchor: crates/ironclaw_reborn_composition/src/attested.rs:135
   No test drives register_attested_gate through the assembled composition.
   Fix: tests::attested::register_attested_gate_seals_grant_and_persists_binding

Conventions (6)

1. Medium Six new env vars not added to .env.example (crates/ironclaw_reborn_composition/src/attested_config.rs:2968-2970, confidence 100) — anchor: CONTRIBUTING.md:135
   Seven new environment variables introduced but none appear in .env.example.
   Fix: Add all new env vars to .env.example.

2. Medium Binding immutability invariant removed — DO NOTHING to DO UPDATE upsert (crates/ironclaw_attested_store/src/binding.rs:24-104, confidence 100) — anchor: CLAUDE.md:58-61 (no diff position — body only)
   Both PG and libSQL backends changed from insert-only to upsert. The immutability regression test is deleted. No rationale provided.
   Fix: Document the rationale for removing binding immutability.

3. Medium SSRF RPC endpoint validation type and all unit tests deleted (crates/ironclaw_chain_signing/src/broadcast_http.rs:7-268, confidence 100) — anchor: CLAUDE.md:58-61 (no diff position — body only)
   The entire RpcEndpoint validated newtype (~170 lines) is removed. Broadcasters now accept raw String URLs with no construction-time validation. No rationale provided.
   Fix: Restore RpcEndpoint validation or document why raw-string URLs are safe.

4. Medium JSON-RPC response validation weakened — body cap, ID check, NEAR hash validation removed (crates/ironclaw_chain_signing/src/broadcast_http.rs:276-310, confidence 100) — anchor: CLAUDE.md:58-61 (no diff position — body only)
   Three defensive checks removed: MAX_RPC_RESPONSE_BYTES, RPC_REQUEST_ID echo check, validate_near_tx_hash. No rationale provided.
   Fix: Restore the body-size cap and ID echo check.

5. Medium Concurrent CAS atomicity test deleted — no coverage for ledger guard under contention (crates/ironclaw_attested_store/tests/ledger_contract.rs:35-122, confidence 100) — anchor: AGENTS.md:80 (no diff position — body only)
   The concurrent advance_to_broadcast_yields_one_winner test is deleted. No rationale provided.
   Fix: Restore the concurrent CAS test.

6. Low hex_encode replaced with format! loop — performance regression over hex crate (crates/ironclaw_attested_store/src/broadcaster.rs:149-155, confidence 75) — anchor: CLAUDE.md:19-20 (no diff position — body only)
   hex_encode changed from hex::encode to a manual format! loop per byte.
   Fix: Restore hex::encode(bytes).

Local Patterns (4)

1. Medium DurableCustody struct and its public fields lack doc comments (crates/ironclaw_reborn_composition/src/attested_durable.rs:161-164, confidence 75) — anchor: crates/ironclaw_reborn_composition/src/attested_durable.rs:187
   DurableCustody has no doc comments on struct or fields.
   Fix: Add struct and field doc comments.
   Also flagged by: maintainability/Low

2. Medium AttestedProofClaim struct missing doc comment despite documented fields (crates/ironclaw_product_workflow/src/attested_continuation.rs:66-66, confidence 75) — anchor: crates/ironclaw_product_workflow/src/attested_continuation.rs:83
   AttestedProofClaim has doc comments on fields but no struct-level doc.
   Fix: Add a struct-level doc comment.

3. Medium AttestedProofKind enum missing doc comment despite documented variants (crates/ironclaw_product_workflow/src/attested_continuation.rs:37-37, confidence 75) — anchor: crates/ironclaw_product_workflow/src/attested_continuation.rs:92
   AttestedProofKind has doc comments on variants but no enum-level doc.
   Fix: Add an enum-level doc comment.

4. Medium AttestedContinuationRejection enum missing doc comment despite documented variants (crates/ironclaw_product_workflow/src/attested_continuation.rs:93-93, confidence 75) — anchor: crates/ironclaw_product_workflow/src/attested_continuation.rs:83
   AttestedContinuationRejection has detailed variant docs but no enum-level doc.
   Fix: Add an enum-level doc comment.

Maintainability (2)

1. Medium assemble_libsql and assemble_postgres are near-identical (crates/ironclaw_reborn_composition/src/attested_durable.rs:179-320, confidence 100) — anchor: crates/ironclaw_reborn_composition/src/attested_durable.rs:199
   Two assembly functions share the exact same control flow.
   Fix: Extract a single generic assemble_stores helper.

2. Low non_empty_env duplicates present_env from attested_config.rs (crates/ironclaw_reborn_composition/src/attested_durable.rs:69-74, confidence 75) — anchor: crates/ironclaw_reborn_composition/src/attested_durable.rs:69
   Two private helpers with the same semantics.
   Fix: Delete non_empty_env and reuse present_env.

[henrypark133]

Medium — assemble_libsql and assemble_postgres are near-identical.

Two assembly functions share the exact same control flow.

Fix: Extract a single generic assemble_stores helper.

[zmanian]

Declining the extraction. assemble_libsql and assemble_postgres look parallel but instantiate different concrete store types (LibSql* vs Postgres*) whose constructors differ in both signature and arg type (Arc<libsql::Database> + connect/new vs deadpool_postgres::Pool + connect/new), and there is no shared trait over (connect, new, run_migrations) to make a single generic helper over. A generic assemble_stores would require introducing a backend-abstraction trait object/seam purely to satisfy the dedup, which adds indirection over backend-specific constructors for marginal benefit. The shared post-build envelope (validate endpoints -> broadcaster -> registry -> assemble) is already the same call sequence and documented as intentionally parallel.

[zmanian]

Fair point on maintainability — but assemble_libsql and assemble_postgres are gated on different backend features (libsql vs postgres) and hand two unrelated DB-handle types (Arc<libsql::Database> vs deadpool_postgres::Pool) to distinct store constructors. A shared generic would need a trait abstracting "open store over handle H" across both backends purely to collapse ~20 lines of otherwise-identical control flow, and each function is only ever compiled under its own feature. Leaving as-is for now (documented, low-value dedup); happy to revisit if a third backend lands and the duplication actually multiplies. Leaving open so it's not lost.

[zmanian]

Addressed @henrypark133 review (commit 4770aac)

Verified each finding against the reviewed commit (ec162d036) before acting. Disposition table:

Security

#	Finding	Disposition
S1	state_secret distinct-byte floor too permissive (<8)	Fixed — raised to MIN_DISTINCT_SECRET_BYTES = 16; a 10-symbol 40-byte key now rejected. Test added. (Strictly more restrictive — preserves fail-closed.)

Bugs

#	Finding	Disposition
B1	is_internal_host misses IPv6 fe80::/10	Fixed — new is_internal_ip covers fe80::/10 (is_unicast_link_local), fc00::/7 (is_unique_local), loopback, unspecified, and unwraps IPv4-mapped (::ffff:a.b.c.d) back through the IPv4 guards (blocks ::ffff:127.0.0.1, ::ffff:169.254.169.254). 9 unit tests + endpoint-level rejection tests.
B2	ValidatedUrl Debug leaks query/fragment	Fixed — redacts query+fragment, keeps scheme+host+path. Test added.
B3	parse_hex returns empty vec for ""/"0x"	Fixed — empty input fails closed as MalformedProof. Tests at helper + caller.

Tests (all added)

map_continuation_error (all 7 variants) · map_attested_continuation_rejection (all 7, status+kind+non-retryable) · NEAR redirect decode (full_access + function_call) · WalletConnect decode (±public_key) · partial-NEAR-config fail-closed + invalid-provider-config fail-closed (env-serialized) · register_attested_gate seals grant and persists binding through the assembled composition.

Conventions / body-only regressions

#	Finding	Disposition
C1	7 env vars missing from .env.example	Fixed — all added with fail-closed docs.
C2	Binding immutability DO NOTHING -> DO UPDATE	Documented (kept). The durable backends were stricter than the trait contract (its in-memory reference is HashMap::insert = last-write-wins); the upsert aligns them. The authoritative one-shot guard is the sealed-grant CAS (threat #1) + signing-ledger (threats #6/#7) over the shared stores — not the binding row, which cannot produce a second sign/broadcast even if overwritten. Rationale now in the binding.rs module doc.
C3	RpcEndpoint SSRF newtype deleted	Documented (moved, not lost). SSRF/metadata-host validation moved up to the single composition-layer seam (validate_optional_rpc_url / is_internal_host, now hardened per B1) where operator RPC URLs are validated once before any broadcaster is built. Avoids two divergent allowlists. Rationale in broadcast_http.rs module doc.
C4	JSON-RPC body cap / ID echo / NEAR-hash check removed	Body cap restored — shared read_jsonrpc_body (MAX_RPC_RESPONSE_BYTES = 64 KiB, content-length + actual-bytes guard) now used by EVM/NEAR/Solana, since the response is a trust boundary. Also fixed a latent non-ASCII panic in decode_hex. The request-id echo / NEAR-hash-shape checks were not restored: id is a fixed local constant and tx-hash shape is validated downstream ([u8;32] / b58 decode); happy to restore if you'd prefer.
C5	Concurrent CAS atomicity test deleted	Restored — advance_to_broadcast_yields_one_winner re-added for the libsql backend (in-process), adapted to the collapsed taxonomy: the lost-CAS path now reports InvalidTransition { from: post-win-state } (the ConcurrentAdvance variant was removed in this PR). Still asserts exactly one winner / 31 fail-closed losers / no double-advance.
C6	hex_encode format! loop perf	Fixed — switched to write! into the preallocated String (no per-byte format! allocation).

Local patterns / maintainability

#	Finding	Disposition
LP1	DurableCustody fields lack docs	Fixed — field docs added (struct already documented).
LP2	AttestedProofClaim missing struct doc	Stale — struct doc present at attested_continuation.rs:59-65 in ec162d036; anchor pointed at field docs.
LP3	AttestedProofKind missing enum doc	Stale — enum doc present at :33-36.
LP4	AttestedContinuationRejection missing enum doc	Stale — enum doc present at :89-91.
M1	assemble_libsql/postgres dedup	Declined — the two functions instantiate different concrete store types with backend-specific constructor signatures and no shared connect/new/run_migrations trait; a generic helper would add a backend-abstraction seam for marginal benefit. Shared post-build envelope is already identical and documented as intentionally parallel.
M2	non_empty_env duplicates present_env	Fixed — deleted non_empty_env, reused pub(crate) present_env.

Verification: affected-crate cargo test green (reborn_composition lib 79 + attested suites; product_workflow full; chain_signing; attested_store ledger incl. restored concurrent test); cargo fmt --check clean; cargo clippy --all-features zero warnings on all five touched crates. (Two pre-existing facade_factory failures — RuntimeProcessPort Missing — predate this commit and are unrelated.)

[henrypark133]

PR13 — Paranoid-Architect Review (attested-signing production wiring)

Summary

This is the final wiring PR of the attested-signing substrate. The core security invariants are handled well — verify-before-resume atomicity, one-shot ledger guards, fail-closed config, SSRF hardening on RPC endpoints, secret-as-SecretString, placeholder detection. Test coverage is thorough. Most of what follows is real concerns, not nits.

---

Issue Table

#	Severity	Location	Issue
1	High	attested_continuation.rs:63	RebornAttestedContinuation holds Arc<LocalDevContinuationDriver> — hardwired to the in-memory monomorphization. When PR13 lands, build_webui_services wires this for every deployment including future non-local-dev ones. The durable assembly helpers (assemble_libsql/assemble_postgres) produce LibSqlAttestedComposition/PostgresAttestedComposition which have a different driver type, so there is no way to wire the durable driver through this port. This is noted as a deferred follow-up but it means the assembly seam will be silently in-memory in any production deployment that flips the runtime guard before the erasure PR lands.
2	High	attested_durable.rs:validate_optional_rpc_url	Errors from RPC URL validation are surfaced as ContinuationError::Broadcast { reason }, which map_continuation_error maps to BackendUnavailable. Startup validation failure therefore looks identical to a runtime broadcast outage. Prefer a distinct startup error type (or at minimum ContinuationError::InvalidArgument) so ops can tell "misconfigured at boot" from "RPC is down".
3	High	reborn_services.rs:broadcast_resolved ordering	After resume_turn succeeds (turn is AttestedResolved, resume guard consumed), a broadcast_resolved failure surfaces as a 503 with retryable: false. This is self-contradictory: the comment says "client can observe the failure category" but a retryable: false 503 gives the client no actionable path. Since the grant and ledger are consumed, a retry will hit the CAS and get a 409 LedgerGuard — but the client sees non-retryable 503 and has no reason to retry. This is an observable stuck state: the turn is AttestedResolved, the grant is consumed, and the broadcast never completed, with no client-visible recovery path.
4	Medium	attested_config.rs:near_from_env	The partial-config wildcard arm uses EmptyUrl as the error variant for a missing state_secret (field: "near_state_secret"). EmptyUrl is semantically wrong for a missing env var — no URL was empty. The error message reads "near_state_secret must not be empty" which is confusing to an operator who has not set the var at all. A new MissingRequired { field } variant (or AttestedConfigError::PartialConfig { missing: field }) would be clearer.
5	Medium	attested_continuation.rs:119	downcast failure on VerifiedAttestedContinuation maps to ProofRejected, which maps to HTTP 400 client error. A type mismatch here is an internal composition wiring error (a different port implementation produced the handle), not a client proof failure. It should map to BackendUnavailable (503) to avoid a wiring bug silently presenting as a proof rejection.
6	Medium	attested_config.rs:PLACEHOLDER_SECRET_EXACT	"near" is in PLACEHOLDER_SECRET_EXACT (exact-match). A 32-byte secret that happens to equal the string "near" fails closed — fine. But "near" is only 4 bytes and never reaches the 32-byte floor check, so this arm is unreachable. More importantly, "test", "key", "secret" are also in the exact list — these are 4–6 byte strings that also never reach the entropy check. The list is unreachable dead code that creates false confidence. Document or remove.
7	Medium	attested_durable.rs:is_internal_host	DNS rebinding is not covered: the SSRF check is purely syntactic (literal IP matching, no DNS resolution). A hostname that resolves to 127.0.0.1 at runtime (e.g. a compromised DNS entry or split-horizon internal DNS) would pass this check. The comment correctly notes this is a TOCTOU/network-call trade-off, but the limitation is not surfaced in .env.example or the error message. Operators need to know they must also validate at the network/firewall layer.
8	Medium	attested_config.rs tests	partial_near_config_fails_closed covers (none,none,none), (base_url_only), (base+callback), (all three). The mirror cases where only state_secret or only callback_url is set are not tested. The wildcard arm in near_from_env does cover them correctly, but the test does not verify this — given this is a security invariant, the missing sub-cases should be explicit.
9	Low	attested_continuation.rs:62	RebornAttestedContinuation is the public export but verify_and_claim / broadcast_resolved silently ignore _scope and _run_id. These parameters were added to AttestedGateContinuationPort presumably for future tenant isolation / audit trail wiring. If they are not used now and not planned to be used, document why. If they are planned, add a // TODO(tenant-audit): propagate scope/run_id to driver for audit trail.
10	Low	Cargo.lock	windows-sys is downgraded from 0.61.2 to 0.60.2 across 11 crates, and one crate (wincolor dependency) gets 0.48.0. Three distinct windows-sys versions in the lockfile is unusual. Confirm this is an intentional resolution from a dependency change in the stacked PRs and not a merge artifact.
11	Nit	attested_durable.rs:assemble_libsql/assemble_postgres	Both functions map all store migration errors to ContinuationError::Broadcast { reason }. This is a startup path (pre-broadcast), and reusing the broadcast error variant obscures where in the boot sequence the failure occurred. Low impact but confusing during incident diagnosis.

---

Positive Observations

• verify_and_sign correctly runs BEFORE ledger row creation: rejected proofs leave no stranded ledger row and no claimed grant — cleaner than the previous "row created, not advanced" approach. The threat-matrix test assertions were correctly updated to reflect LedgerError::NotFound instead of Approved.
• StateSecret uses SecretString + explicit expose_bytes() accessor; Debug and the NearRedirectConfig Debug both redact the secret. The placeholder entropy checks are conservative and well-documented.
• ValidatedUrl::Debug redacts query/fragment, protecting API keys that may appear in RPC URLs.
• map_continuation_error exhaustively covers all ContinuationError variants with a coverage test — good regression protection against new variants.
• BackendUnavailable correctly maps to 503 (not 400) for chain-signing/broadcast failures.
• SSRF IPv4-mapped IPv6 (::ffff:127.0.0.1) unwrapping is a good hardening detail.
• with_clean_attested_env serializes env-mutation tests through a mutex — correct for process-global env.

---

Action Required

Items 1 and 3 need resolution before merge:

Item 1: Either gate the build_webui_services wiring behind a local-dev check (consistent with the existing build_reborn_runtime guard) so a future production flip cannot accidentally take the in-memory path, or document the production invariant more prominently than a // deferred comment in attested_durable.rs.

Item 3: Decide on the stuck-state semantics for broadcast_resolved failure after a successful resume_turn. Options: (a) make BackendUnavailable retryable (retryable: true) with a note that the retry will hit LedgerGuard if the broadcast actually completed; (b) add a separate BroadcastPending state to the ledger that the client can poll; (c) document the stuck state explicitly in a follow-up issue. The current behavior (non-retryable 503 after the one-shot is consumed) leaves the system in a silent stuck state.

Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com

[zmanian]

Addressed: paranoid-architect review (2026-05-26)

New tip 223163b89. Per-item disposition below. Build + cargo test + clippy (all-features) green on the three touched crates (ironclaw_attested_runtime, ironclaw_reborn_composition, ironclaw_product_workflow); the only remaining test failures are the pre-existing RuntimeProcessPort Missing cases (#4085), confirmed to fail identically on the unmodified PR head.

#	Disposition
1 (High, in-memory driver hardwired)	Documented invariant. RebornRuntime holds a concrete LocalDevAttestedComposition field and build_reborn_runtime rejects every non-local-dev profile — there is no type through which build_webui_services could wire a durable driver, and no production RebornRuntime can be constructed today. Strengthened the doc on build_webui_services + RebornAttestedContinuation to state this prominently and warn against relaxing the profile guard before the trait-erasure follow-up.
2 (High, startup vs runtime error)	Fixed. Added ContinuationError::Config; RPC-URL validation failures in the durable assembly now surface as Config, not Broadcast/BackendUnavailable.
3 (High, stuck-state after resume)	Documented; declined retryable:true with rationale. On a broadcast-stage failure the driver moves the ledger to the Unknown terminal state — the on-chain outcome is genuinely unknown. retryable:false is deliberate: auto-retry would risk double-submission of a tx that already landed. Recovery is operator-mediated reconciliation; a pollable BroadcastPending state is the driver's existing deferred cross-crate follow-up. Documented at both the call site and the error mapper.
4 (Medium, EmptyUrl for missing secret)	Fixed. Added AttestedConfigError::MissingRequired { field }; the partial-NEAR-config arm now reports it (the absent var was never present, and state_secret is not a URL).
5 (Medium, downcast → 400)	Fixed. Handle-type-mismatch now maps to BackendUnavailable (503), not ProofRejected (400) — a wiring bug must not look like a client proof failure. Still fails closed (no broadcast).
6 (Medium, dead placeholder list)	Fixed. Removed PLACEHOLDER_SECRET_EXACT: every entry is shorter than the 32-byte length floor, so it could never fire. Documented why; substring markers + distinct-byte floor cover the real >=32-byte degenerate cases.
7 (Medium, DNS rebinding)	Documented. Added a note to .env.example that the SSRF check is syntactic-only (no DNS resolution) and operators must also enforce egress allow-listing at the network/firewall layer.
8 (Medium, missing partial-config sub-cases)	Fixed. Added explicit fail-closed tests for the only-state_secret, only-callback, missing-callback, and missing-state_secret permutations, asserting MissingRequired with the correct field.
9 (Low, unused scope/run_id)	Fixed. Added a TODO(tenant-audit) note explaining they are reserved for the per-tenant audit-trail wiring and intentionally unused today (the driver derives authority from the gate_ref-keyed binding).
10 (Low, windows-sys lockfile)	Triaged, no action. Multiple windows-sys versions are a benign transitive-resolution artifact (Windows-only; this is a darwin/Linux project), not a source regression from this PR.
11 (Nit, migration errors use Broadcast)	Fixed alongside #2: durable store-connect/migration failures now use ContinuationError::Config.

All hardened invariants preserved: SSRF guard (fe80::/10, fc00::/7, loopback, IPv4-mapped, fail-closed), validated fail-closed provider config, durable binding store in production assembly, one-shot CAS, WYSIWYS, ship-gate, openssl-free.

[abbyshekit]

Code review — feat(signing): register NEAR/WC providers + flip production to durable composition (attested-signing PR13)

Multi-agent review (security · bugs · performance · tests · conventions) at 223163b8. Diff-only mode.

Independent review of an open PR.

2 findings — 1 High, 1 Medium. Posted as a comment (advisory). Confidence ≥ 50, deduplicated across reviewers.

Sev	Conf	Reviewer	Location	Finding
High	90%	bugs	attested_config.rs:230	state_secret distinct-byte floor (>=16) false-rejects valid CSPRNG hex secrets, hard-failing startup on the NEAR-provider path
Medium	70%	tests	runtime.rs:704 (in body)	Production-flip safety invariant (build_reborn_runtime rejects non-LocalDev) has no regression test

Findings without an inline anchor (cited lines fall outside the diff hunks)

[Medium · 70%] Production-flip safety invariant (build_reborn_runtime rejects non-LocalDev) has no regression test

crates/ironclaw_reborn_composition/src/runtime.rs:704-711 — tests reviewer

This PR flips production toward the durable attested composition and rests its entire "signing never runs in-memory in production" safety argument on one guard: build_reborn_runtime returns InvalidArgument for any profile that is not LocalDev (runtime.rs:704). The PR itself makes this load-bearing in the new webui.rs doc comment: "this path cannot silently take the in-memory composition in a production deployment ... Do not relax the build_reborn_runtime profile guard ... or this seam would become silently in-memory in production." RebornRuntime.attested_signing is typed concretely as the in-memory LocalDevAttestedComposition (runtime.rs:174), and build_reborn_runtime is the only constructor that wires it (and the only caller of build_attested_composition). factory.rs::build_production_shaped returns substrate-only RebornServices and never builds a RebornRuntime, so the line-704 check is the sole gate.

Every test in the runtime.rs mod tests block calls build_reborn_runtime with a LocalDev profile (all 14 local_dev_runtime_* / send_user_message_* tests). The rejection branch (lines 705-710) is happy-path-only: no test passes Production or MigrationDryRun and asserts InvalidArgument. I checked the runtime.rs test module, the factory.rs inline tests, and all three tests/*.rs files in this crate (attested_provider_registration.rs, attested_durable_assembly.rs, attested_gate_resolve_ingress.rs) — none exercise the non-LocalDev path. This is precisely the failure mode the repo's testing.md "Test Through the Caller" rule warns about: a predicate (profile == LocalDev) gates a side effect (which attested composition is wired), and if someone weakens or deletes if !matches!(profile, RebornCompositionProfile::LocalDev), every existing test still passes while production silently gains an in-memory, restart-losing signing composition. The rest of PR13's new surface (AttestedProvidersConfig::from_env, build_provider_registry, assemble_libsql/postgres, validated_endpoints/SSRF host classifier, proof decode + error mapping) is comprehensively tested through the caller; this guard is the one uncovered invariant.

Fix: Add tests::runtime::build_reborn_runtime_rejects_non_local_dev_profile covering the case that a RebornRuntimeInput whose services.profile() is RebornCompositionProfile::Production (and again MigrationDryRun) returns Err(RebornRuntimeError::InvalidArgument { .. }) and never constructs a runtime — pinning the guard that keeps the in-memory LocalDevAttestedComposition out of production. A unit/async test in the existing mod tests that builds the same RebornRuntimeInput the local_dev_runtime_* tests use, swaps the profile to Production, and asserts the error variant is sufficient.

---

_{Generated by near-ai-code-review (5 parallel reviewer agents + intent analysis). Diff-only; confidence ≥ 50; ≤ 15 inline comments.}

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (2)

• staging
• reborn-integration

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d74d56ae-3de3-4208-afd1-37fce99e798a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[zmanian]

Superseded by #6811, part of consolidating the 20-PR attested-signing-* stack into 8 PRs against current main.

Verified before closing: this branch is an ancestor of the old stack tip, so its content is carried forward in full. The consolidation also re-based everything onto current main — which had moved 79 commits and retired ironclaw_product_workflow, the WebUI composition facade, and the old failure-kind vocabulary — and fixed the CI failures that were red here, including the missing [package.metadata.ironclaw] layer declarations that had been blocking the bottom of the stack.

Closing to keep the queue honest. The review history stays on this PR and remains readable; reopen if the consolidation is rejected.