Conversation
Add the connector-side half of full-knowledge vault-inbox delivery: a baton/vault-inbox/v1 recipient that seals a connector's plaintext credential into an existing C1 vault-inbox submission using only shipped library primitives and the shipped Latchkey reader. - EncryptionConfig gains vault_inbox_recipient_config (arm 102) carrying the frozen binding coordinates and the public inbox JWK, plus VaultInboxSuite / VaultInboxConfigVersion enums. CredentialIssueOptionDescriptor gains vault_inbox_profiles (tag 11) so a connector advertises the profile and C1 never dispatches an unadvertised one. - New provider emits the exact SecretSubmissionPayloadV3 container, seals it under Base / X-Wing / HKDF-SHA256 / ChaCha20-Poly1305 with info == aad == the Latchkey injective framing, and returns the submission envelope JSON in EncryptedData.encrypted_bytes with key_ids = [inbox_key_id]. - Validation before the provider runs: profile/suite, bounded identifiers, non-zero generation, public AKP JWK of exactly 1216 bytes with no private material, re-derived thumbprint, and a low-order X25519 probe. The vault-inbox recipient must be the only config, and issuance must yield exactly one plaintext value. - docs/vault-inbox-delivery.md freezes the wire contract, and the pinned Go-produced fixture in testdata is what the existing Rust reader opens. Supersedes the abandoned baton/full-knowledge-vault/v1 envelope and the native-CEK-import / age-to-FK designs; the existing age and JWK providers are unchanged. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
Regenerate Go, opaque, and validate bindings for the vault_inbox_recipient_config EncryptionConfig arm, the VaultInboxSuite / VaultInboxConfigVersion enums, and CredentialIssueOptionDescriptor.vault_inbox_profiles. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
Write the regenerated fixture with 0600 and wrap the fixture note, both flagged by go-lint on the PR. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
Address every finding from the PR review. No blocking issues were raised; these are the gaps the existing instruments did not reach. - Bound the whole payload, not just the value. display_name and description are sealed into the submission container, so an oversized string would mint a credential whose submission the inbox could never accept. Both are bounded to the submission row's own limits, and the sealed envelope is checked against the inbox's 2 MiB cap before it is handed back. - Pin payload_scheme to latchkey.vault_submission.secret.v1. The provider only ever emits SecretSubmissionPayloadV3, and the scheme is bound into the HPKE AAD, so accepting any other label would let a producer seal a payload the reader attributes to a scheme it does not carry. - Refuse only unknown fields on the inner config. EncryptionConfig is shared with every other provider; refusing unknown fields there made it non-extensible for all of them, for a property only the frozen binding actually needs. - Move the exclusivity check into NewEncryptionManager, and the one-plaintext rule into an exported manager method. ValidateEncryptionConfigs is not called by RotateCredential or CreateAccount, and the registered-action path encrypts a whole plaintext list, so both could otherwise seal several complete submission envelopes bound to one submission id. - Enforce the capability advertisement: a vault-inbox recipient whose profile the selected descriptor does not list is now refused, the same way an unadvertised key profile is. - Pin the binding bytes in-repo against a literal transcribed from the Latchkey framing, so the fixture is no longer only an assertion about this package's own output. The cross-language proof remains the Rust test referenced in docs/vault-inbox-delivery.md. Verified: go build ./... clean; go test ./pkg/crypto/... ./pkg/connectorbuilder/... ./pkg/actions/... all pass. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…path Documentation corrections first, because the previous wording overstated what the transport completes: - Registration is INTERMEDIATE. A submission reaching PENDING_REVIEW is durable and reviewable, not DELIVERED and not a native secret. - The transport is keyless: between the connector's output and the registered submission the ciphertext is copied byte-identically and is never decrypted, resealed, or re-encoded. - Member ingestion is a separate authorized slice: an authorized C1 vault member decrypts, creates the native secret through the ordinary secret path, and records acceptance. Only that, plus the recorded acceptance, is DELIVERED, and only then are the destination's secret_id/version_id populated. - Ordinary authorized reveal is a mandatory acceptance test for that slice, not a production prerequisite of delivery. Sharing is separate again. - Failure compensation is now stated as exact actions: revoke exactly the provider credential the issuance created, clean up exactly that submission and its exact native version if one exists, preserve siblings, and never mint a replacement to cover the failure. - The thumbprint is JWK validation against the recipient JWK, not an independent HPKE info/AAD field; submission_id and content_type are authenticated by living inside the sealed payload. - Pre-mint gates (config, capability advertisement, exclusivity) are now separated from post-mint checks (output cardinality, name/description bounds, envelope size), because only the first half can refuse an issuance before a credential exists. - The config is C1 authority carried on the authenticated action transport, and the SDK does not verify recipient attestation signatures. - The wire profile is unchanged: bytes, field numbers, suite, framing and envelope stay exactly as specified. Tests through the real builder, with a counted mint. IssueCredential runs the registered provider and the result is inspected: - a valid config, an advertised profile, and one usable output mint exactly once and return one sealed EncryptedData with the vault-inbox provider and key id, and no plaintext anywhere in the response; - unknown config version or suite, an unsupported payload scheme, an unknown inner field, a mismatched provider or thumbprint, an unadvertised profile, and mixed or duplicate recipient configs are all refused with zero mint calls; - zero, multiple, unnamed, or oversized provider values mint exactly once, then fail with no partial result and no second mint. Removing any one gate flips a call count in these tests, so they fail on a regressed boundary rather than passing quietly. Also silences go-lint's G101 false positive on the payload scheme constant, which is a protocol label rather than a credential. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Regenerated from the source comment change on CredentialIssueOptionDescriptor.vault_inbox_profiles. No wire change. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
nonamedreturns rejected the test helper's named return values. No behaviour change. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- RotateCredential and CreateAccount now apply the one-value cardinality rule before they fan plaintext across the recipients. They build the manager directly, so the exclusivity check alone left a lone vault-inbox config free to seal two complete submission envelopes bound to one submission id. - Drop IsVaultInboxProvider: an exported helper with no caller is a permanent commitment to nothing. - Rename the seal call site's local from `info` to `kdf`. The KDF and AEAD are the suite; the fourth argument is the binding. Naming it `info` in the one file whose contract is `info == aad == binding` invited exactly the wrong reading. - Correct a stale comment that still claimed the thumbprint is bound inside the HPKE binding. It is not: the binding frames the key id and generation, and the thumbprint is a JWK re-derivation check. - Make two gate tests actually pin their gates. The mixed-recipient case used an invalid age recipient, so the age validator refused it before the exclusivity gate ran; it now uses a valid one. The mismatched-provider case routed to the age provider by name, so the vault-inbox provider's own branch was unreachable from the request path; that branch is now pinned by a direct provider test. - Correct the NewEncryptionManager comment, which overstated the exclusivity check as covering the rotate/create paths entirely. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… path The cardinality rule was applied to CreateAccount as "exactly one plaintext", but only a SuccessResult is expected to carry one: AlreadyExists, ActionRequired, and InProgress legitimately return none, so the rule turned "the account already exists" into a hard FailedPrecondition and discarded the structured result. Split the rule into its upper and exact forms: - ValidateVaultInboxPlaintextCardinality stays "exactly one" for the paths whose contract requires a value: issuance and the registered-action path. - ValidateVaultInboxPlaintextCardinalityAtMostOne permits zero and refuses more than one. CreateAccount uses this one: a non-success outcome with no plaintext now returns its structure unchanged, while two plaintexts are still refused because they would seal two complete envelopes bound to one submission id. Covered by TestVaultInboxCreateAccountKeepsStructuredResults, which drives the real CreateAccount through a fake account manager: an AlreadyExists result with no plaintext returns its structure and zero encrypted data, and two plaintexts fail without re-invoking the manager. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The relaxation in the previous commit was result-type blind, so it was wider than the contract its own doc stated: a SuccessResult carrying zero plaintexts returned success with empty EncryptedData, meaning the vault-inbox submission was never sealed and nothing reported it. The rule now follows the result: a SuccessResult requires exactly one value, and the non-success results — which carry none by contract — take the upper bound only. Two values are refused on either path, because they would seal two complete envelopes bound to one submission id. The helper's doc now says which callers may use it, so a caller whose contract requires a value cannot reach the permissive rule by accident. Covered: a Success result with no plaintext now fails, alongside the existing AlreadyExists-passes-through and two-plaintexts-refused cases. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A SuccessResult does not always carry a credential: with NoPassword or Sso the connector legitimately creates the account and returns success with no plaintext. The exactly-one rule therefore failed *after* the account existed, turning a real account into a failed CreateAccount whose retry only sees AlreadyExists. Refuse the combination up front instead, next to the credential-option conversion and before the create, so the account is never made: a vault-inbox recipient exists to deliver a value, so pairing it with an option that yields none is a misconfiguration rather than a runtime failure. The post-create exactly-one rule stays as the connector-contract check for the options that do ask for a value. Both halves are pinned by call count: an empty success under RandomPassword is refused after one create, and NoPassword is refused with zero creates. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…n errors Three review findings, all real: - EncryptedPassword does not ask the connector to produce a value either: it carries material the caller already holds. With a non-empty list the option conversion already refuses it, but with an empty list it slipped past the gate, reached the connector with nothing set, and yielded no plaintext — exactly the post-create failure the gate exists to prevent. The allow-list is now RandomPassword only. - RotateCredential never called the gate, so the same asymmetry existed there and worse: the connector rotates first, and only then does the cardinality rule fail on zero values, leaving the prior credential invalidated with nothing delivered. The gate now runs before the rotation. - The new checks used `if err := ...`, shadowing the function-scoped `err` that the deferred EndSpanWithError reads. The RPC failed while the trace reported a clean span and no message. Both the new option gate and the cardinality check now assign the outer err; the same shadowing at the rotate call site is fixed with it. Covered by TestVaultInboxRotateRefusesBeforeMinting: NoPassword, Sso, EncryptedPassword, and unspecified options are each refused with zero rotations, and a password-producing option still rotates once and seals one envelope. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The config gate ran only on the issuance and action paths, so on rotate and
create an unsupported vault-inbox config version, suite, payload scheme,
thumbprint, or JWK was first checked inside Provider.Encrypt — after
manager.Rotate had already invalidated the prior credential, or after the account
existed. That contradicted the config-version comment and the documented pre-mint
list. Both paths now call ValidateEncryptionConfigs alongside the option gate.
Also:
- IssueCredential still shadowed the function-scoped err that the deferred
EndSpanWithError reads, so those two refusals ended the span with no error
status. Converted to assign the outer err.
- parsePublicKey no longer uses DisallowUnknownFields. The thumbprint check
beside it canonicalizes over {alg, kty, pub} and tolerates extra members, so
refusing them made the parse stricter than the contract it validates against:
a served JWK carrying kid, use, or key_ops re-derived the same thumbprint and
was then rejected. Private material is still refused explicitly.
Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Four review findings on the previous push: - The config pre-check was not scoped to vault-inbox, so it changed CreateAccount and RotateCredential for every recipient type: an unresolvable config used to surface only inside Encrypt, which never runs when there is nothing to encrypt, and now failed earlier. It is gated on HasVaultInboxConfig so other recipients keep their existing behaviour. - ValidateEncryptionConfigs' doc still claimed it did not affect create/rotate; it now names its callers and their conditions. - Requiring RandomPassword is right for create, but a rotation with no options at all is a supported shape — the connector mints its own replacement — so the rotate path gets its own rule that accepts unset and still refuses the options that can produce nothing. - The registered-action cardinality gate had no test, and it is the one place the manager could be nil: it is only assigned when the handler declares secret return types. TestRegisteredActionVaultInboxCardinality pins 0/1/2 values and the no-secret-return-types early return. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…low test - IsVaultInboxConfig now also matches the provider name, not only the inner message. Routing keys on `provider` first, so a config naming this provider with the inner message unset still reaches this provider; it skipped the new scoped pre-checks and failed at Encrypt instead — after the account existed or the credential had been invalidated. It failed closed, but late, which is the outcome those gates exist to prevent. - The registered-action two-value case never reached the cardinality gate: both values were named api_key, so the duplicate-name check returned first and the subtest would have passed with the gate deleted. The values now use distinct declared names, and both cardinality cases assert on the gate's own message so an unrelated check cannot satisfy them. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The provider-name branch of IsVaultInboxConfig was untested: every vault-inbox fixture in the suite sets both the provider and the inner message, so deleting that branch left the suite green — the same hollow shape as the previous round. TestVaultInboxGateMatchesProviderNameOnlyConfig adds a config that names this provider with the inner message unset and asserts both CreateAccount and RotateCredential refuse it with the provider call count still zero, which is the outcome the branch exists to produce. While verifying that claim by mutation I found a second gap: deleting the cardinality rule from RotateCredential also left the suite green, so the rotate path's post-mint refusal was untested. A two-value rotate case now asserts the refusal is the cardinality rule and that the rotation is not retried. Both were confirmed by removing the corresponding line and watching the tests fail, then restoring it. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A mutation sweep over every vault-inbox gate in this PR found one further decorative test: deleting the `len(plaintexts) > 1` bound from ValidateVaultInboxPlaintextCardinalityAtMostOne left the suite green, because the only two-value create case used a SuccessResult and therefore went through the exactly-one rule instead. A non-success result carrying two plaintexts is now covered, asserting the refusal is the at-most-one rule and that the account manager is not re-invoked. Sweep results, each verified by deleting the gate's condition, requiring the targeted test to fail, and restoring the file: - exclusivity rule, exclusivity in NewEncryptionManager, create and rotate option rules, exactly-one cardinality, advertisement gate, config gates on create and rotate, provider-name branch, rotate cardinality: all detected - at-most-one upper bound and rotate cardinality: were decorative, now detected The sweep harness itself had a false-negative mode worth recording: a mutation that fails to compile reads as "not detected", which is how the advertisement gate was first misreported. Its mutation is now written to compile, and it is detected by two tests. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Section 6 described only the issuance path, so it read as if every path used exactly-one cardinality and as if the credential-option rule did not exist. It now states which pre-mint gates run on which path, that the config-shape check is scoped by the inner message or the provider name, and the credential-option rule for create (RandomPassword only) and rotate (RandomPassword or unset). The post-mint cardinality rule is now a per-path table, because the contracts genuinely differ: a CreateAccount SuccessResult requires exactly one value, while its non-success results — which carry none by contract — allow zero and preserve their structured result. Two or more is refused on every path. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…plit The pre-mint table added in the previous commit listed the cardinality rule as a pre-mint gate for registered actions, which contradicts the section's own "issuer calls = 0" heading: that check runs after handler.invoke has returned, so the action has already executed and anything it provisioned externally exists. A connector author could have read the row as a refusal that happens before the action runs. The row now says config shape only, with a pointer to the post-mint rule. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e scrub claim Three review findings on the provider: - public_jwk_json was the one config field with no bound, handed straight to two JSON parses. It now goes through maxJWKBytes (16 KiB; a valid AKP JWK is about 1.7 KB), so this gate is uniformly bounded rather than leaning on the gRPC message cap. - The canonical JWK the thumbprint is taken over was assembled by interpolating pub without JSON escaping, so a pub containing a quote, backslash, or control character hashed a string that is not the form the Latchkey side derives. Marshalling a typed struct keeps it valid JSON for any input while producing identical bytes for every legitimate key — TestPublicKeyThumbprintMatchesLatchkeyVector still matches the documented digest, which is what proves the value is unchanged. - clear(payload) read as "the credential is scrubbed after sealing", but it only zeroes the slice json.Marshal returned: the pooled encodeState keeps an equal copy of value_b64, and the base64 string is immutable. The comment now says best-effort over the copies this function owns and names what cannot be cleared, rather than implying a guarantee the code does not provide. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The case added in the previous commit was hollow: it used a non-JSON padding string, so publicKeyThumbprint's decoder refused it regardless of maxJWKBytes and the assertion held with the bound deleted. The replacement pads a *real* JWK with an extra member. The thumbprint still matches and the key still parses, and since parsePublicKey no longer rejects unknown members the size bound is the only thing that can refuse it. Verified by deleting the bound and watching the test fail, then restoring it. A same-key-at-the-bound assertion is included so a failure cannot come from the padding member itself. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
make protogen (buf generate) over ece97e9's proto comment changes. Purely descriptive text in generated code -- no field, tag, or type changes.
Committed before any implementation, as the high-risk process requires. The plan names what can actually fail and what each check proves. It states the load-bearing claim plainly -- that the JWK-configured provider emits byte-identical inbox wire bytes to the arm-102 path at ae9de8d given the same randomness -- and then bounds where that can break: strict extension parsing refusing malformed, unknown, duplicate, and trailing input before any provider invocation; the missing/wrong-provider downgrade path; key_id versus a conflicting JWK kid; and the legacy JWK and age regressions that must not move. It also records the gaps rather than hiding them: the C1 Rust ingestion and reveal lane is out of scope and will not be claimed as covered by Go tests, recipient attestation verification is not implemented, and every negative case gets mutation-checked against a harness that has previously proven gates *run* without proving they *refuse*. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
The first commit put the plan in a plausible place rather than the required one. docs/BUG_CATCHING.md and the precedent at docs/verification/encrypted-action-results/ require the implementation-blind plan at docs/verification/<change>/plan.md with evidence.md beside it, frozen on first commit. A plan reconstructed from a finished diff is not preregistration, so placement and shape are load-bearing rather than cosmetic. Plan now follows the precedent's shape -- Risk, Contract, Coverage model, Criteria, Instruments, Change orders -- with twelve criteria, a coverage model that names the executable dimensions, and the mutation check called out as the instrument for the negative rows rather than a formality. Two corrections folded in from the repository rules, both of which change the implementation rather than the write-up: - .claude/skills/ci-review.md requires deprecate-first and reservation for proto removal, and requires a proto source change to carry its generated pb/ output in the same change. The plan now records the decision as reservation of tag 102 informed by the baseline check, rather than assuming deletion is available. - The tag-102 baseline is recorded: absent from origin/main and from v0.31.0, present only on the PR branch, so the breaking answer differs by baseline and both answers are stated. evidence.md opens with every criterion NOT RUN and states the gaps up front, including the previously observed failure mode of gates that were proven to run but not to refuse. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
Replaces the dedicated config arm with the existing JWK carrier: the provider name selects inbox mode, JWKPublicKeyConfig.pub_key holds the recipient JWK, and EncryptionConfig.key_id is the single authoritative inbox key id. Binding context travels in a namespaced extension member on that JWK. Parsing is strict, because the extension is a protocol surface and Go's decoder resolves a repeated member by keeping the last one, which would let two readers of the same bytes disagree about the key or the binding. Malformed input, a repeated member in the outer JWK or the extension, an unknown extension member, trailing content after either object, a wrong version or suite, non-string or null JWK members, and private material are all refused before any provider work. Ordinary optional JOSE metadata is still accepted, so the strictness is scoped to the protocol extension rather than to all of JOSE. The extension deliberately does not carry the inbox key id. Two sources for that value could disagree about which key a ciphertext is bound to, so key_id stays the only one. The thumbprint canonicalization is unchanged and covers alg/kty/pub only. The extension is outside it, so adding or removing context cannot move a legitimate key's thumbprint. bindingBytes now takes the validated binding context rather than a config message. It depends on the values, not on where they were parsed from, which is what makes this path able to produce byte-identical output to the arm it replaces. Recognition is by provider name alone. A JWK public key config without this provider is ordinary classical encryption and is not classified as inbox mode, and a config that is malformed-but-present is still recognized so it is rejected as a bad vault-inbox config rather than silently downgraded. Proto: field 102 and its name are reserved rather than reused, and the message and the config-version enum it used are removed. VaultInboxSuite stays because capability advertisement on CredentialIssueOptionDescriptor still depends on it, so the capability gate now asserts the one profile this selector corresponds to. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
make protogen (buf generate) over the source commit. Generated separately from the proto source, per the repository's commit ordering. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
The capability gate now reads the profile from vaultinbox.AdvertisedSuite rather than from a config field, so the package needs the import. Production code builds clean after this. The test files in pkg/crypto, pkg/crypto/providers/vaultinbox, pkg/actions and pkg/connectorbuilder still construct the removed config message and do not compile yet; updating them to build the JWK carrier is the next slice. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
The recipient config moved from VaultInboxRecipientConfig to a JWKPublicKeyConfig whose pub_key is the recipient JWK, with the binding context in the baton_vault_inbox extension and the inbox key id on EncryptionConfig.key_id. The tests still built the removed message. Migrate every constructor and accessor to the typed JWK helper, keeping each existing assertion and the committed interop vector. The binding and tamper cases now run through recipientFromConfig or build the binding context directly, so they pin the parsed coordinates rather than a protobuf message. Two parser fixes fall out of preserving the existing assertions: - Unknown fields on the shared EncryptionConfig stay tolerated, as the old config-message case asserted, while unknown fields on the provider-specific JWK config are refused. - A JWK kid that disagrees with the authoritative key_id is refused, so two sources cannot disagree about which key a ciphertext is bound to. Tests: go test ./pkg/crypto/... ./pkg/actions/... ./pkg/connectorbuilder/... Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
The capability-advertisement comment still pointed at VaultInboxRecipientConfig, which this alternative removes. Say what the field means instead of naming a message no longer in the tree. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
Add the focused cases the frozen plan calls for: strict extension and outer-JWK parsing, provider and key-id mismatch, duplicate members, unsupported and missing fields, private material, and the pre-mint position. Strengthen the two criteria that needed a direct instrument: the thumbprint ignores the extension (C3) and an extended JWK with no provider is refused rather than sealed under another profile (C5). Re-run the mutation matrix over every guard. Sixteen guards are isolated by a failing test when disabled; six are shadowed by a sibling guard in the same chain and are recorded as gaps rather than coverage. Update docs/vault-inbox-delivery.md with the JWK-configured config surface and its tradeoffs, and record the results, the coverage reduction, and the change orders in docs/verification. Tests: go test ./pkg/crypto/... ./pkg/actions/... ./pkg/connectorbuilder/... Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| // Checked after the plaintext list is known and before any encryption, so a | ||
| // vault-inbox recipient cannot receive two whole submission envelopes. | ||
| if err := encryptionManager.ValidatePlaintextCardinality(plaintextData); err != nil { |
Contributor
There was a problem hiding this comment.
🟡 Suggestion (confidence: medium): For registered actions, this is the only vault-inbox cardinality check, and it runs after handler.invoke has already executed the action. An action that declares two or more requiredSecretReturnNames can never satisfy a vault-inbox recipient, but it still runs its side effects (for example, minting a credential) and then fails here. The issuance path has a pre-mint gate (validateVaultInboxProfileAdvertised). Consider rejecting at dispatch next to ValidateEncryptionConfigs (~line 915) when crypto.HasVaultInboxConfig(encryptionConfigs) && len(handler.requiredSecretReturnNames) > 1.
Comment on lines
+417
to
+422
| // 102 held VaultInboxRecipientConfig. This alternative carries the inbox | ||
| // binding context in a typed extension member on the recipient JWK instead, so | ||
| // the arm is removed and its number and name stay reserved rather than being | ||
| // reused or silently dropped. Inbox mode is selected by the provider name on | ||
| // this message, never inferred from the key type. | ||
| reserved 102; |
Contributor
There was a problem hiding this comment.
🟡 Suggestion (confidence: high): On main, field 102 was never assigned, so "102 held VaultInboxRecipientConfig" is only true of the unmerged #1145. The same "this alternative" PR-history wording also appears at line 432 and in vaultinbox/jwkconfig.go (AdvertisedSuite, recipient), and it ships in the generated pb output. Reserving 102 is harmless, but consider deleting the history narration or cutting it to a neutral note such as // Reserved; do not reuse.
Contributor
General PR Review: IGA-4417: Reuse JWK config for X-Wing vault inbox encryptionBlocking Issues: 0 | Suggestions: 2 | Threads Resolved: 0 Review SummaryI scanned the full PR diff for security and correctness. The review covered the new Risk triage (per
Security IssuesNone found. Correctness IssuesNone found. Suggestions
Prompt for AI agents |
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
1 participant
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.
Summary
Alternative to #1145 for review; this does not replace or modify that PR.
EncryptionConfig.JWKPublicKeyConfig,provider, andkey_idfor the vault inbox recipient. Carry destination and delivery context in a versionedbaton_vault_inboxJWK extension instead of a dedicated config message.privmember. Explicit empty content type still normalizes togeneric.The public-key thumbprint is a consistency check, not independent vault-key attestation. Destination authority remains the authenticated C1 action transport. C1 would need to construct the alternative config; no C1 or Rust changes are included here.
Test plan
go vet, targeted crypto/actions/connectorbuilder tests, protobuf generation with no diff, and Buf lint/format passed.See
docs/vault-inbox-delivery.mdfor the contract anddocs/verification/fk-inbox-jwk-alternative/evidence.mdfor scoped evidence and gaps.