Give every PAM failure a stable, machine-readable code - #8235
Draft
Hinton wants to merge 27 commits into
Draft
Conversation
Emitting a cipher's full secret data now requires a FullCipherAccess witness, and the default response shape is partial. A path that forgets to obtain a witness returns partial data - a visible bug - rather than leaking secrets. - PartialCipherData.Strip reshapes the plaintext JSON envelope, keeping the encrypted name (and, for logins, the encrypted URIs) and dropping every other encrypted field. Nothing is decrypted; retained values stay individually-encrypted EncStrings. - FullCipherAccess is the witness. Its factories are internal, so only the leasing gate can mint one; the Full* response ctors call Require() per element, keeping bulk lists fail-closed rather than only single reads. - Each of the 4 cipher response types gains a Full* subclass deriving from its partial counterpart, so ListResponseModel holds a polymorphic mix and the wire contract is unchanged (verified: the generated OpenAPI spec adds and removes no schemas). All secret setters are now protected. - Attachment metadata is withheld from partial responses because it carries each attachment's encryption key, and GetAttachmentData is gated: a download URL grants the encrypted attachment, which the caller could decrypt with an org key they already hold. - ICipherLeaseGate is the decision point, with NoopCipherLeaseGate as the OSS default. The interface deliberately has zero PAM-domain dependencies, so this lands independently of the leasing domain; the commercial gate overrides the registration later in startup. Only the web vault understands the partial shape, so gated ciphers are omitted entirely for every other client rather than sent partial. An older client would render an item with no credentials as though it were empty, and saving it back would overwrite the withheld fields with the blanks the client holds; dropping the item is the lesser harm, and it stays visible in the web vault where the user can request access. PartialCipherSupport is the single predicate, and it fails safe - an absent or unrecognized device type is treated as unable to render the shape. In sync this mirrors the existing FilterUnsupportedCipherTypes pattern; on single reads a gated cipher is a 404. Behavior is unchanged: the no-op gate authorizes everything, so every existing response is still full and nothing is ever filtered. The bulk read paths use the gate's self-loading overload rather than eagerly querying collections, so nothing extra is queried while the feature is off. Adds a reflection-based fitness guard asserting the invariants that make this fail closed - no public setter on any secret property, no way for application code to mint a witness, and every Full* ctor requiring one - so a future refactor that reopens one of those holes fails a test.
`PartialCipherData.Strip` now serializes a purpose-built DTO with `IgnoreWritingNullAndCamelCase` instead of round-tripping `CipherLoginData`. This matches the shape the SDK's restricted decrypt path consumes (the same `LoginUri` fields as a full login) and drops the redundant singular `Uri` the legacy computed getter leaked. Input is parsed case-insensitively so the stored PascalCase blob and an already-stripped camelCase blob both round-trip. Adds a JsonDocument shape test pinning the camelCase wire contract.
ICipherLeaseGate and NoopCipherLeaseGate live in the Core project but declared `Bit.Pam.Services` — the root namespace of the separate Pam.Domain project (`Bit.Pam`). A type's namespace should say which project it is in, so both move to `Bit.Core.Pam.Services`, matching the `Bit.Core.<Area>.Services` shape every other Core area uses.
TryAddScoped contradicted the override contract in the comment beside it: TryAdd* means "only if absent", so it reads as though the default should win, while the commercial gate is meant to replace it. Plain AddScoped is what AddOosServices already uses for commercially-overridden defaults such as IProviderService. The comment now states the invariant that actually matters, which is at the override site: the commercial registration must be a plain Add* and must run after AddBaseServices. A TryAdd* there would no-op against this default regardless of which verb is used here, leaving leasing ungated.
`new CipherMiniResponseModel(...)` silently meant "partial": the shape rode in on a `partial: true` default that the type name said nothing about, so a call site emitted reduced data without ever saying so. Each of the four models is now an abstract base with `Partial*` and `Full*` subclasses, and the boolean is gone. A call site has to name the shape it emits, and one that names neither does not compile. `Data` and `PartialData` stay declared on the base, so all four schemas serialize exactly as before — verified against the generated internal OpenAPI spec: the same four cipher schemas, each still carrying `data`, `partialData` and `attachments`, and no `Partial*`/`Full*` schema leaked in. `PopulatePartialData` mirrors `PopulateFullData`, which is what withholds attachment metadata now — no more `partial ? null : ...` in shared construction. The concrete types are sealed, so neither shape can be extended into a third meaning, and the fitness tests assert that plus the naming rule over the whole assembly rather than a hand-listed pair per family. There is no `PartialCipherMiniDetailsResponseModel`: every path building the mini-details shape is authorized org-wide rather than through collection membership, so nothing reaching it is gated. Making the bases abstract surfaced a live regression this PR had introduced: `DeleteAttachment` built a bare `CipherMiniResponseModel`, so *every* caller — gated or not — got a response with no `data` and no attachments after deleting an attachment. Both delete-attachment endpoints now build the shape deliberately: the admin variant unrestricted, the member variant through the gate.
`FilterUnsupportedCipherTypes` and `FilterGatedCiphersForUnsupportedClients` answered the same question — what can this client render? — and were only separate because the gated half needs the witness, which needs the caller's collections loaded further down. The gate now decides over all the user's ciphers and a single `FilterCiphersUnsupportedByClient` applies both rules at one call site. Passing the unfiltered set to the gate costs nothing: it computes authorization in memory and still queries nothing while the flag is off.
`Bit.Core.Services.IFeatureService` is obsolete in favour of `Bitwarden.Server.Sdk.Features.IFeatureService` (BWA0002). Fully qualified at the injection point, matching the already-migrated controllers, since `Bit.Core.Services` is imported for its other types. The sibling deprecations flagged in review — `ResponseModel` and `ListResponseModel` — are deliberately left alone: `Bit.HttpExtensions` `ListResponseModel<T>` constrains `T` to the `Bit.HttpExtensions` `ResponseModel`, so taking them here would mean rebasing the whole cipher response hierarchy, and with it sync, emergency access and export, onto the new base. That is its own migration, not a drive-by inside a security change.
`NoopCipherLeaseGate` reads as inert, but it authorizes full secret data for every cipher. For a security control that is worth saying outright, so it is now `UnrestrictedCipherLeaseGate` — matching `FullCipherAccess.Unrestricted()` and the interface's own `Unrestricted()`. This departs from the repo's `Noop*` convention for OSS stubs deliberately.
Two gaps reviewers hit. The gate's parameters read as organization-specific, but it is called with whatever the caller holds, so implementations must decide for user-owned ciphers too — a user-owned cipher is reachable through no collection and therefore never gated, which an implementation must authorize rather than withhold for want of an organization. And the nullable collection parameters had no documented meaning: null means "not loaded, because the caller has no organizations" and is equivalent to empty. `FullCipherAccess` now carries the division of labour it exists to enforce: the controller decides and obtains the witness; a response model never decides, it only shapes what the witness permits — reducing one cipher, or dropping the uncovered ones from an aggregate such as an export.
Dead since before this PR and it was warning about it.
The summary above it already says the gate authorizes every cipher.
A static transform holding two private DTOs, for what is really one shape:
{name} is {name, uris} with uris omitted, and the serializer already drops
nulls. Collapsed into a single type, so the property list is the whole
allowlist — one place to read when auditing what can escape a gated cipher —
and Strip deserializes straight into it rather than parsing CipherLoginData
and copying two fields out. It still does not derive from CipherData, so the
legacy computed singular Uri getter and the base fields stay out of the
envelope.
Output is unchanged, checked byte-for-byte against the previous implementation
over 13 inputs: login with and without URIs, null name, explicit empty uris
array, already-stripped camelCase, each non-login type, a non-login carrying
stray URIs, and empty/whitespace/null blobs. PartialCipherDataTests pass
unmodified.
One trade-off worth knowing: a non-login's URIs are now cleared by a line of
code rather than by the DTO having nowhere to put them, so that exclusion is a
statement instead of a structural guarantee.
…model Export was the only aggregate where the response model decided which ciphers made it into the payload; sync, emergency access and the cipher lists all filter in the controller and let the model shape what it is handed. Moving the `Where` up makes the rule hold everywhere: controllers decide and filter, response models shape by the witness and never drop. It also fails louder. The model now hands every cipher it receives to `FullCipherMiniDetailsResponseModel`, so a path that forgets to filter throws on the witness check instead of quietly shipping a shorter backup.
The witness test was written three ways — `fullAccess.Authorizes(id)`, `fullAccess?.Authorizes(id) == true`, and `is not null && Authorizes(id)` — at the branch that decides whether secrets go out. One of those getting inverted leaks, and only the path it sits on would notice. A static `From` on each abstract base now owns that test, so it appears once per family and nowhere else. Call sites state what they want and get the shape the witness permits; the five casts to the base type and the controller's `BuildCipherMiniResponse` helper are gone with it. Paths authorized out of band still construct their `Full*` directly, which keeps `_cipherLeaseGate.Unrestricted()` visible at the admin and export call sites rather than hidden behind a factory.
The witness test was written three ways — `fullAccess.Authorizes(id)`, `fullAccess?.Authorizes(id) == true`, and `is not null && Authorizes(id)` — at the branch that decides whether secrets go out. One of those getting inverted leaks, and only the path it sits on would notice. A static `From` on each abstract base now owns that test, so it appears once per family and nowhere else. Call sites state what they want and get the shape the witness permits; the five casts to the base type and the controller's `BuildCipherMiniResponse` helper are gone with it. Paths authorized out of band still construct their `Full*` directly, which keeps `_cipherLeaseGate.Unrestricted()` visible at the admin and export call sites rather than hidden behind a factory.
Stage 1 of two. Copies the final-state files from pam/poc-rebased to their pam/uat paths with no adaptation, so the next commit's diff shows every deviation from the POC. Does not compile on its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… handlers
Stage 2 of two: adapts the POC blobs to pam/uat. Diffing this commit against its
parent shows every deviation from pam/poc-rebased.
The three handlers now orchestrate the ported queries and commands instead of
throwing NotImplementedException, so GET /leases/mine, /leases/active,
/access-requests/mine and /leases/ciphers/{id}/state return data rather than 500.
Adaptations:
- Restore the API enum shim as the wire contract. The domain's AccessRequestStatus
has no Activated member, so mapping it straight to the wire would decode a Denied
request as Activated. DomainEnumMapping derives Activated from ProducedLeaseId.
- Register a no-op IAccessAuditEventEmitter in the Pam service project. It cannot
live in Core, which has no Pam.Domain reference. Every command injects it, so the
registration is load-bearing even though it records nothing.
- Ship the two approver/requester notifiers as no-ops: the push types they would
send do not exist on this branch.
- Drop the approver email. Its ICollectionRepository.GetManagingUserIdsAsync sproc
and IMailService overload are both absent here.
- Widen AccessRequestResult with the automatic decision so the submit response
carries the decision log its published contract promises.
- Keep uat's response-model shape (parameterless ctor, mutable properties) and add
a domain-taking ctor alongside, preserving the generated OpenAPI schemas.
The only wire change is AccessRequestStatus gaining Activated and shifting the
values after Approved, which aligns the spec with the already-published
sdk-internal binding. All 18 PAM paths and the other 24 PAM schemas are unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Write gating was deliberately out of scope for the UAT endpoint port, which left a leasing-gated cipher fully editable: a member holding no lease could edit, delete, restore, re-file, or attach to a credential whose secrets the read gate was already withholding from them. The read gate returns partial data for such a cipher, so a save would also have written the client's blanks over the fields the server suppressed. Add EnsureCanMutateAsync and EnsureCanMutateManyAsync to ICipherLeaseGate, no-op them in UnrestrictedCipherLeaseGate so OSS and flag-off behaviour is unchanged, and call them from the twelve CipherService mutation paths. Also gate CiphersController.PutPartial, which writes straight to the repository and so never reaches the service-level gate — leaving it open would let a caller re-file one gated cipher at a time while MoveManyAsync refused the batch. Brand-new ciphers, skipPermissionCheck, and org-admin paths stay ungated, for the same reason the read gate mints an unrestricted witness for a context already authorized out-of-band. Refusal is NotFound so a write attempt cannot confirm that a credential the caller cannot reach exists. The bulk decision honours leases where the bulk read deliberately does not: a read copies secrets into every client's local store for as long as that store lives, whereas a write copies no secret anywhere, so refusing the holder's own edit would withhold nothing. It refuses the whole batch when any cipher is gated, because DeleteManyAsync has no per-item result channel and a partial success would silently diverge from what the client believes happened. The bulk path resolves the governing rule per cipher, so a flag-on bulk mutation costs a query per cipher. A structural pre-filter over the caller's collections would remove that but is not sound: UserCollectionDetails filters on Organization.Enabled and CollectionCipher_ReadByUserId does not, so a disabled organization would clear a cipher the resolver gates. Batching belongs behind IGoverningRuleResolver instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The append-only store the audit trail is written to and read back from: the AccessAuditEvent table and its two stored procedures, a consolidated migration for MSSQL plus generated ones for the EF providers, and the Dapper and EF repositories behind IAccessAuditEventRepository. Rows are self-contained. AccessAuditEvent_Create snapshots the actor, requester, cipher, collection, and rule display names into the row at write time, so the trail read touches no other table and a later rename or delete cannot rewrite history. The subject ids are deliberately not foreign keyed for the same reason -- an event outlives what it references. Only OrganizationId is, so the rows go when the organization does. The EF path resolves those names in C#, because JSON_VALUE -- which the procedure uses to read the cipher name out of its encrypted Data document -- has no portable EF translation. This is the persistence layer only; nothing consumes it yet. The emitter that writes to it and the trail endpoint that reads from it are separate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Wires the commercial PAM library to the audit store: the emitter every
state-changing command already calls now appends to it, and the trail is read
back through GET organizations/{orgId}/audit, authorized by AccessEventLogs so
whoever can read the organization's event logs sees the whole trail regardless
of collection management.
The read collapses each action's before/after pair, which share a correlation
id, into one row -- the Outcome when it landed, otherwise the lone Attempt,
which the response flags as in-doubt rather than dropping. Emission is not
transactional by design, so an Attempt with no Outcome marks an interrupted
action instead of a silently lost event.
NoopAccessAuditEventEmitter goes with this: the interface is commercial-only, so
the placeholder had no remaining caller once the real emitter was registered.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The three access-rule commands never emitted, so creating, editing, or deleting a rule left no audit event -- even though RuleCreated/RuleUpdated/RuleDeleted were defined and the write payload carried AccessRuleId and RuleName for exactly this purpose. Unlike the expiry and credential-access kinds, these three were never marked deferred; they were simply unwired. Each command now emits the Attempt/Outcome pair the rest of the module does. Create and update take the actor from the LastEditedBy the handler already stamps. Delete had no actor at all, so DeleteAsync takes the caller's id -- the delete is hard, which makes the audit event the only surviving record of who did it and what the rule was called. That is also why RuleName is captured from the row before the delete rather than joined at write time. The create's Attempt cannot name the rule: Repository.CreateAsync assigns the id, so before the write there is no rule to name. Both create and update hold their Outcome until the collection links are written too, so an Attempt with no Outcome flags a half-applied change rather than reading as a clean one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AccessRule.DefaultLeaseDurationSeconds and MaxLeaseDurationSeconds were write-only: both round-tripped through create/update and rendered in the admin console's rule form, but nothing at request time ever read them. The only cap applied was the hardcoded 24h global in SubmitAccessRequestCommand, so a rule configured for 15 minutes granted a 1 hour lease in full (PM-39858). LeaseDurationBounds now owns the arithmetic that folds a rule's optional bounds together with the global ceiling. It has two callers that have to agree exactly -- the pre-check publishes the bounds so a client can shape its duration picker, and submit enforces them -- because a client narrowing to a cap the server does not enforce is how the rule's maximum came to be ignored in the first place. GoverningRule (and the resolver that builds it) now carry both fields; it previously copied only the extension-related ones, leaving the two lease-duration fields unreadable downstream. Submit applies the effective cap on both paths: the automatic duration, and the human-approval window, which is pinned at submit and so has no later gate of its own. AccessRuleWriteValidator additionally rejects non-positive durations and a default above the rule's own maximum. The edit form already couples its two pickers, but a write straight to the API bypassed that and could persist a rule whose every pre-filled request exceeded its own cap. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`TypedResults.BitwardenValidationProblem` hard-coded 400. A failure can be carried by the same body without being bad input — a state conflict is a 409 — and a caller that reads only the status should still learn that much before it reads the codes. Additive: the parameter defaults to 400, so every existing call site is unchanged.
Contributor
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the conversion of the PAM surface from thrown exceptions to Code Review Details
Considered and dismissed:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| /// </summary> | ||
| Task<AccessRequestResult> SubmitAsync(Guid userId, Guid cipherId, AccessRequestSubmission submission); | ||
| /// <returns> | ||
| /// The submitted request, or the failure that stopped it: <see cref="Errors.AccessRequestNotFound"/> when the |
Contributor
There was a problem hiding this comment.
🎨 SUGGESTED: The cref names AccessRequestNotFound, but SubmitAsync returns CipherNotFound for this case.
Details and fix
SubmitAccessRequestCommand.SubmitAsync returns new CipherNotFound() when _cipherRepository.GetByIdAsync(cipherId, userId) comes back null — and CipherNotFound is documented in AccessRequestErrors.cs as exactly this case ("The cipher does not exist, or the caller cannot see it").
Suggested change
| /// The submitted request, or the failure that stopped it: <see cref="Errors.AccessRequestNotFound"/> when the | |
| /// The submitted request, or the failure that stopped it: <see cref="Errors.CipherNotFound"/> when the |
No behavioural difference (both are NotFoundError and render the same 404 envelope), but this <returns> block is the contract an SDK author reads to enumerate the error surface, so the wrong record name is worth correcting.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## pam/uat #8235 +/- ##
==========================================
Coverage ? 63.88%
==========================================
Files ? 2431
Lines ? 106107
Branches ? 9571
==========================================
Hits ? 67786
Misses ? 36029
Partials ? 2292 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This was referenced Aug 20, 2026
PAM rejected with `ErrorResponseModel`, which carries a human-readable message and no discriminant. Every failure a client must act on differently was therefore identified by matching the server's English sentence, and the web client had grown two catalogs of eighteen sentences matched with `String.includes()` to do it. Three of those are not failures at all — already-active, already-approved and already-pending mean the requester has what they asked for, and the UI reconciles rather than reporting an error — so rewording one sentence would silently turn a reconciliation into a red toast, with no test failing on either side. PAM now answers with RFC 7807 problem responses in the shape the Admin Console's invite-link confirm endpoint already ships: the stable code is `errors.<property>[].type`, and the property is the request field a form should mark invalid, or `code` when no single field is at fault. The message is unchanged and travels as `detail`. To get there, PAM commands stop throwing for expected failures and return `CommandResult<T>` carrying an `Error` from `Bit.Services.Pam.Errors` — one record per failure, each with the code and the property on it. Handlers render them through the single `PamErrorResult` arm of their `Results<…>` return type, which keeps the success schema in the generated OpenAPI. A coded failure is a 400 unless it is a `ConflictError`, which is a 409. The model-state filter is deliberately left alone. A DataAnnotations rejection means the request never bound cleanly, which leaves a client nothing to act on differently, so those failures carry no code and keep the `ErrorResponseModel` 400 the controllers produced. The guards behind those attributes that do carry a code — a blank name, a non-positive duration — are the commands defending their own contract, not a second user-facing validation layer. Codes are for the semantic failures: state a request conflicts with, a bound it exceeds, a rule that denies it. A `NotFoundError` deliberately keeps the `ErrorResponseModel` 404 that a thrown `NotFoundException` produces — PAM's read paths still throw those, so all its 404s stay identical, and a 404 needs no code because the status already tells it apart. Codes exist for the failures a status cannot separate. The exception filter stays as the safety net for genuinely unexpected throws. Contract: a code is never localized and never reworded once shipped; adding one is additive, since a client treats an unknown code as a generic failure.
Hinton
force-pushed
the
pam/error-codes
branch
from
August 20, 2026 10:41
0a579ef to
8d44cb9
Compare
Hinton
marked this pull request as draft
August 24, 2026 07:20
Hinton
force-pushed
the
pam/uat
branch
5 times, most recently
from
August 27, 2026 15:24
1194de6 to
1bdc1f7
Compare
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
3 participants
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.
🎟️ Tracking
Companion to the client-side ask in
docs/pam-server-asks.md(clientspam/uat). Base branch ispam/uat, notmain.📔 Objective
PAM rejected with
ErrorResponseModel— a human-readablemessageand no machine-readable discriminant. Every failure a client must act on differently was therefore identified by matching the server's English sentence. The web client had grown two such catalogs, eighteen sentences matched withString.includes(), written weeks apart by different people, both carrying the same note in their own words: "When the server grows a code, this catalog is the single place to retire."Three of the eighteen are not failures at all.
access_already_active,access_request_already_approvedandaccess_request_already_pendingmean the requester already has what they asked for; the UI reconciles — collapses the form, re-reads the access state, shows an informational toast. Reword one of those sentences today and a reconciliation silently becomes a red error toast, with no test failing on either side. Status codes don't disambiguate: they were all400.This gives every PAM failure a stable code, in RFC 7807 problem responses.
The shape
The same shape the Admin Console's invite-link confirm endpoint already ships (
TypedResults.BitwardenValidationProblem), so clients learn one parser:errors.<property>[].type— stable, never localized, never reworded once shipped.reason,durationSeconds,collections,name, …), named exactly as the request model serializes it. A failure no single field caused — a state conflict, a request shaped for the wrong approval mode, a denial by the governing rule — is keyed bycode.detailis today's message, unchanged."type": "conflict_error"). The code is still what a client switches on; the status is a coarse hint for anything that reads no further. This is a status change for the three reconcile cases, which were 400.How
PAM commands stop throwing for expected failures and return
CommandResult<T>carrying anError— reusing the Admin Console's v2Error/IValidationError/CommandResulttypes rather than inventing a parallel set. One record per failure inBit.Services.Pam.Errors, with the code and the property on it:Handlers render them through the single
PamErrorResultarm of theirResults<…>return type. Declaring the union rather than returning bareIResultis what keeps the success schema in the generated OpenAPI, and so in the SDK's bindings.The model-state filter converted too, with the failing DataAnnotations attribute as the code (
required,range,string_length) — otherwise a client would still need a second parser for anything rejected before a handler ran, and[Required] Reasonon the extension model would shadowextension_reason_requiredentirely.Deliberately unchanged
ErrorResponseModel. PAM's read paths still throwNotFoundException, so all its 404s remain identical however they were reached. A 404 needs no code — the status already tells it apart. Codes exist for the failures a status cannot separate.Scope
All ~35 expected failures across the PAM surface, not just the eighteen the clients match today: submit, activate, cancel, decide, extend, revoke, the access-rule writes, and the three rule-engine denial reasons (which the catalogs never covered, so they were indistinguishable prose). One error style across the whole surface means the leasing UI's next prose-matching catalog never gets written.
TypedResults.BitwardenValidationProblemgained an optionalstatusCode(defaults to 400) for the 409s — additive, every existing call site unchanged.Tests
PamErrorResultTestsasserts the bytes, not the result type — the code and its placement in the body are the promise.PamErrorCatalogTestsreflects over the catalog: codes are snake_case, a code reused across two errors keys the same property, the set of deliberately-shared codes is written down, and the eighteen the clients match today are all present. Two integration tests prove the shape survives the real pipeline end-to-end.427 unit + 31 integration tests pass.
Downstream
The SDK maps codes onto typed error variants (
AccessRequestError::AlreadyPending, …) and both client catalogs are deleted rather than copied into the next client. Those are bitwarden/sdk-internal#1400 and bitwarden/clients#22544, also targetingpam/uat.