feat(prs-556): choose the ABI trust posture at deploy time

[pepe-anchor]

Why

PRS-556, part 2 of 2. Part 1 (#422) has merged, so this now targets main directly and is no longer stacked.

#406 made the parser accept caller-supplied ABI mappings with no signature instead of dropping them. That fixed a real bug, but it left acceptance of unsigned ABIs as the parser's unconditional default: any caller can omit the signature and get an unverified ABI decoded and rendered.

@prasanna flagged the posture itself as designed wrong, and the visibility mechanism #406 added does not work. The aggregated warn! per request is never seen by a signer, because Turnkey doesn't surface parser logs today. So the trust decision is both implicit and unauditable.

This moves it to where the deployer chooses it and the signer can check it: the parser's cmdline, which for the enclave binary lands verbatim in the pivotArgs of the manifest the operators approve.

What

Two mutually exclusive postures on parser_app:

--accept-unsigned-abis                       # today's behaviour, now an explicit opt-in
--accept-signatures-from-pubkey <hex>        # only allowlisted signers; unsigned is rejected. Repeatable

Neither flag and it refuses to start. Both and it refuses to start. There is no default: coming up in a mode nobody chose is the failure this replaces.

Threaded ParserConfig -> parse() -> create_registry() -> EthereumVisualSignConverter::with_policy, using the MetadataTrustPolicy type from #422.

tvc-deploy deploy enforces the same XOR and appends the chosen flag to pivotArgs, so the posture is part of the signed manifest rather than something a caller influences per request.

It also fails fast on the two ways the posture could still go wrong between choosing it and the enclave honouring it:

• workflow_dispatch took a free-text app_id but let the posture default to blank, and blank meant --accept-unsigned-abis. Dispatching against a non-test app without filling the field would quietly deploy the permissive posture, which is the class of implicit choice this PR exists to remove. Dispatch now refuses to run without a stated posture, and takes the literal unsigned to opt into accept-unsigned on purpose. The PR-label path has no inputs context and only ever targets the dev TEST app, so it keeps the permissive default.
• A malformed pubkey used to reach the manifest unchecked, and nothing looks at it until parser_app decodes it at startup. A typo cost a consensus round and an enclave that never reported healthy. tvc-deploy now validates the key before writing the manifest: SEC1 shape, then a real point decode via k256. Accepted prefixes are 02/03/05 at 33 bytes and 04 at 65. 05 is SEC1's compact tag and k256 accepts it via decompact, so rejecting it would have blocked a deployment whose key parser_app would have honoured. k256 was already in that crate's lockfile transitively, so the on-curve check costs one line and no new supply chain.

parser_grpc_server takes the same flags but keeps accept-unsigned as its default with a startup line saying so. It's the non-attested dev server, there's no manifest to audit the posture against, so requiring the flag would only break local dev without buying the property.

Existing launch paths (integration harness, src/Makefile, the tvc-deploy workflow) now pass --accept-unsigned-abis explicitly, so behaviour is unchanged. The choice is just no longer implicit.

From the review pass

The merge that brought main in resolved four files to this branch's side, superseding #422's per-request AbiExtraction / rejected_by_policy / unverified counters. Nothing outside those files consumed them, but the merge also took five tests with it, and that part was not deliberate:

• test_register_installs_require_signed_posture is back. It was the only test that went through ChainPlugin::register, and without it, editing register back to EthereumVisualSignConverter::new() compiles clean, leaves the suite green, and silently drops parser_cli to accept-unsigned. Confirmed by making that exact edit and watching the restored test fail.
• Two surviving tests had stopped testing what they name. test_try_extract_invalid_address_skipped had been switched to the require-signed policy while its fixture stayed unsigned, so the posture dropped the entry before the address was ever parsed: it passed with address validation removed entirely. test_accept_unsigned_does_not_enforce_signer_identity signed with the dev seed, which authorized_abi_signers() allowlists under cfg(test), so it would have held even if identity were being enforced.
• create_registry only pinned the accept-unsigned direction, so a version that ignored config.abi_trust outright would still have passed. The require-signed mirror now fails unless the posture reaches the converter.
• The transport layer covered two of the four posture and signature combinations, leaving "gates on signer identity" unproven against a real process. All four are covered now.
• Five comments in cli_plugin.rs still described the CLI's dev-key signature as buying "verified rather than logged as unverified". That outcome no longer exists, and under the CLI's require-signed posture an unsigned entry is dropped outright, so the signature is load-bearing rather than bookkeeping.

MetadataTrustPolicy is now #[non_exhaustive], so a third posture can be added later without breaking downstream matches. Every match on it lives in signing.rs; consumers only construct variants, so nothing is forced into a wildcard arm.

Docs were the part this PR had invalidated without touching. create_registry gained a required argument while docs/wallet-integration/library/getting-started.mdx still showed the zero-arg form in two snippets, so the published snippet no longer compiled. The dApp page was worse than stale: it said the parser accepts any signature from any key and advised shipping unsigned ABIs until the signing pipeline stabilised, which against a require-signed deployment silently degrades rendering instead of easing a rollout. The gRPC server page presents that binary as a self-hosted deployment with a Dockerfile CMD and k8s spec that pass no arguments, and never mentioned a posture existed.

Deploy coupling

tools/tvc-deploy is not a member of the src/ workspace, so it has no compile-time link to the parser: a tvc-deploy change alone goes green in CI while still being a live hazard. Landing the pivotArgs flag before the binary that accepts it means qos_core sees an unknown cmdline arg and panics at startup. The flag and the binary that understands it ship together here, which is why the deploy tooling was not split out further.

One follow-up lives outside this repo: the weekly deploy driver in the pepe-parser-deploy plugin builds the tvc-deploy deploy ARGV with no posture flag, so it will abort at argument parsing once this lands. It fails closed, so there is no misdeploy risk, but it needs a matching change before the next weekly deploy.

Testing

The end-to-end evidence is four integration tests that send an identical request to parser_app instances differing only in cmdline. The ABI describes frobnicate(uint256,address), a selector no compiled-in visualizer knows, so decoding depends purely on whether the caller ABI was honoured:

test accept_unsigned_abis_deployment_decodes_unsigned_abi ... ok            # renders the decoded call
test require_signed_abis_deployment_drops_unsigned_abi ... ok               # falls back to raw hex
test require_signed_abis_deployment_decodes_allowlisted_signed_abi ... ok   # signed by the allowlisted key
test require_signed_abis_deployment_drops_foreign_signed_abi ... ok         # valid signature, wrong signer

The last two are what prove the strict posture gates on signer identity rather than merely on a signature being present.

The signed fixtures are checked in rather than produced at test time. Signing in the integration crate means depending on visualsign-ethereum/dev-signing, and because integration is a workspace member, Cargo would then unify that feature ON for parser_app in any workspace-wide invocation, which is what the keep-out-of-prod note on that feature warns against. ECDSA here is RFC6979 deterministic, so the fixtures are stable, and the regeneration recipe is in a comment beside them.

Unit coverage for deploy-time pubkey parsing (valid, compressed/uncompressed canonicalization, invalid key errors at startup rather than silently shrinking the allowlist), the parser_app flag XOR, tvc-deploy's pivotArgs assembly, the abi_metadata posture behaviour, and the pubkey check (02/03/05/04 accepted, truncated and non-hex rejected, wrong length for the prefix rejected, well-formed hex that is off the curve rejected).

Full suite:

LINT_EXIT=0    make lint   (clippy -D warnings, incl. parser_cli + diagnostics variants)
TEST_EXIT=0    make test   (39 suites, 1492 tests, 0 failures)
FMT_EXIT=0     cargo fmt --all -- --check

tools/tvc-deploy separately: 64 tests pass, clippy and fmt clean. Also checked parser_app under --no-default-features and each of the ethereum/solana/sui/tron features in isolation, since config.rs is feature-gated.

The workflow's posture selection was exercised for every input and event combination rather than just read:

workflow_dispatch + blank      -> hard fail, "abi_signer_pubkey is required"
workflow_dispatch + "unsigned" -> --accept-unsigned-abis
workflow_dispatch + pubkey     -> --accept-signatures-from-pubkey <key>
PR label + blank               -> --accept-unsigned-abis   (dev TEST app only)
PR label + pubkey              -> --accept-signatures-from-pubkey <key>

Manual smoke on the arg handling:

$ tvc-deploy deploy --app-id a ...
error: the following required arguments were not provided:
  <--accept-unsigned-abis|--accept-signatures-from-pubkey <HEX_PUBKEY>>

$ tvc-deploy deploy ... --accept-unsigned-abis --accept-signatures-from-pubkey 04ab
error: the argument '--accept-unsigned-abis' cannot be used with '--accept-signatures-from-pubkey <HEX_PUBKEY>'

$ tvc-deploy deploy ... --accept-signatures-from-pubkey 04aa
error: --accept-signatures-from-pubkey must be a 33-byte (02/03/05-prefixed) or 65-byte
       (04-prefixed) hex secp256k1 public key, got "04aa"

$ parser_grpc_server --accept-signatures-from-pubkey nothex
Error: "invalid ABI trust config: invalid secp256k1 public key: 'nothex'"

Rollback

Needs a redeploy, not just a revert. qos_core's parser rejects unknown cmdline args, so an older parser_app started against a manifest that still carries --accept-unsigned-abis panics at startup.

To roll back: revert the code, then create a deployment whose pivotArgs are back to just --host-ip / --host-port. No state, schema, or wire-format changes otherwise.

Same reason live deployments need a redeploy to pick this up: the flags live in the manifest, so there's no way to flip a running deployment (and, by design, no way to flip it per request).

Backwards compatibility

No proto or payload changes. Every existing launch path keeps its current behaviour, explicitly.

Three intentional incompatibilities, all the point of the change rather than side effects:

• parser_app refuses to start without a posture instead of guessing.
• workflow_dispatch on the tvc-deploy workflow now needs abi_signer_pubkey filled in, either a hex key or the literal unsigned. The PR-label path is unaffected.
• create_registry takes a &ParserConfig. In-repo callers are updated and the published snippets now match.

Not in scope

Solana IDL mappings have the identical problem (extract_idl_mappings accepts unsigned IDLs unconditionally, same per-request default). Kept this ABI-only per the ticket. MetadataTrustPolicy is chain-neutral so extending it is mechanical, worth its own ticket.

PRS-555 is still open and unaffected. Worth restating what it means here, since it bounds what the strict posture buys operationally: a mapping dropped by the posture produces a payload byte-identical to a request that supplied no mappings at all, and the only signal is a log::warn! in a binary that declares no log dependency and initialises no logger. So a wallet that rotates its ABI signer degrades every transaction from decoded fields to raw calldata, silently. The outcome is content-safe rather than wrong, but it is not diagnosable from the response, and closing that needs a response-surface change rather than a parser change.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR makes caller-supplied Ethereum ABI trust a deploy-time choice instead of an implicit/per-request default by introducing a chain-neutral MetadataTrustPolicy, threading it through parser_app configuration and registry construction, and ensuring deployment tooling/workflows bake the chosen posture into TVC pivotArgs for out-of-band auditability.

Changes:

• Introduces visualsign::signing::MetadataTrustPolicy and threads it through parser_app::config::ParserConfig into Ethereum converter construction.
• Updates tvc-deploy and GitHub workflows to require/encode an explicit ABI trust posture at deploy time and validate signer pubkey shape early.
• Adds/updates unit + integration tests to pin the behavior difference between permissive vs strict deployments.

Reviewed changes

Copilot reviewed 22 out of 23 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tools/tvc-deploy/src/main.rs	Adds explicit deploy-time ABI trust flags, pivots args assembly, and SEC1 pubkey shape validation.
tools/tvc-deploy/README.md	Documents the required ABI trust posture flags and their semantics.
src/visualsign/src/signing.rs	Introduces MetadataTrustPolicy and helpers for accept-unsigned vs require-allowlisted-signer.
src/parser/grpc-server/src/main.rs	Threads ParserConfig into parse calls and adds minimal CLI parsing for ABI trust posture.
src/parser/grpc-server/Cargo.toml	Adds visualsign dependency to reference MetadataTrustPolicy.
src/parser/app/src/service.rs	Stores deploy-time ParserConfig in the processor and passes it into parsing.
src/parser/app/src/routes/parse.rs	Extends parse API to accept &ParserConfig and uses it for registry construction.
src/parser/app/src/registry.rs	Threads config into registry creation and pins Ethereum converter posture via with_policy.
src/parser/app/src/lib.rs	Exposes new config module.
src/parser/app/src/config.rs	New deploy-time configuration module + posture resolution and validation.
src/parser/app/src/cli.rs	Adds parser_app CLI posture flags and enforces XOR + non-empty selection at startup.
src/Makefile	Updates local parser_app run target to pass --accept-unsigned-abis explicitly.
src/integration/tests/parser.rs	Adds end-to-end tests proving the same request behaves differently based solely on deployment flags.
src/integration/src/lib.rs	Updates integration harness to always start parser_app with an explicit ABI trust posture.
src/examples/library_integration_test.rs	Updates example to construct registry with an explicit permissive posture.
src/chain_parsers/visualsign-ethereum/tests/lib_test.rs	Updates tests to use with_policy and a strict “require-signed” helper converter.
src/chain_parsers/visualsign-ethereum/src/lib.rs	Replaces allowlist-only model with MetadataTrustPolicy and updates constructors/call sites.
src/chain_parsers/visualsign-ethereum/src/cli_plugin.rs	Makes CLI use strict posture with allowlisted dev signer key (consistent with CLI signing behavior).
src/chain_parsers/visualsign-ethereum/src/abi_metadata.rs	Makes unsigned-acceptance posture-driven; signature integrity always verified when present; adds deploy-time allowlist parsing.
src/Cargo.lock	Records dependency graph update for grpc-server → visualsign.
CLAUDE.md	Documents the deploy-time ABI trust posture model and flags.
.github/workflows/tvc-deploy.yml	Forces explicit posture for workflow_dispatch deployments; defaults remain for PR-label TEST path.
.github/workflows/stagex.yml	Updates deployment manifest guidance to include required ABI posture args.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[pepe-anchor]

Superseded by stacked PRs #440-#444 (stack #445). Same changes, split into reviewable layers:

• feat(prs-556): make ABI trust posture an explicit policy #440: MetadataTrustPolicy type
• feat(prs-556): wire deploy-time ABI trust posture into parser_app #441: parser_app CLI wiring
• feat(prs-556): integrate MetadataTrustPolicy into Ethereum converter #442: Ethereum converter integration
• feat(prs-556): enforce ABI trust posture in tvc-deploy and grpc-server #443: tvc-deploy + grpc-server
• test(prs-556): add integration tests and docs for deploy-time posture #444: integration tests + docs

[Copilot]

🟡 Changes recommended

There are a couple of correctness/robustness issues to address (notably tvc-deploy pubkey validation/manifest formatting not trimming whitespace consistently, and a brittle parser_app CLI test assertion tied to an internal error-variant string).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (3)

tools/tvc-deploy/src/main.rs:491

• validate_signer_pubkey rejects otherwise-valid keys if they include leading/trailing whitespace, but the runtime allowlist parsing (signer_allowlist_from_hex(entry.trim())) trims. This makes tvc-deploy deploy stricter than parser_app for the same input and can cause avoidable deploy failures from copy/paste artifacts.

fn validate_signer_pubkey(hex_str: &str) -> Result<()> {
    let stripped = hex_str
        .strip_prefix("0x")
        .or_else(|| hex_str.strip_prefix("0X"))
        .unwrap_or(hex_str);

tools/tvc-deploy/src/main.rs:333

• pivot_args forwards signer pubkeys verbatim into pivotArgs. If the pubkey string contains leading/trailing whitespace (e.g. quoted env var), it will be recorded in the manifest even though the parser trims during decoding. Trimming here keeps the manifest canonical and consistent with runtime parsing.

    for key in &args.accept_signatures_from_pubkey {
        pivot.push("--accept-signatures-from-pubkey".to_string());
        pivot.push(key.clone());
    }

src/parser/app/src/cli.rs:253

• This assertion depends on an internal error-variant name (MutuallyExclusiveInput) from qos_core's CLI parser, making the test brittle across dependency updates. It's enough to assert parsing fails, or (if you want a message check) assert the error mentions the conflicting flags instead.

        assert!(err.contains("MutuallyExclusiveInput"), "error: {err}");

• Files reviewed: 26/29 changed files
• Comments generated: 0 new
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.