Release: develop -> main - #4577
Merged
Merged
Conversation
…covery path (#4575) * Cover super-roles in the elevated-role assignment guard The write-side check that forbids assigning a gated role to an account without a verified name compared against KycGatedRoles directly. A super-role such as SuperAdmin satisfies every gate through the role hierarchy without appearing in that list, so the highest privilege could still be assigned to an account with no identification signal. Match the clearance query and compare against rolesSatisfying(KycGatedRoles) instead. Cover the super-role case in the test, and construct the service under test with typed mocks so a future constructor change fails to compile rather than wiring dependencies into the wrong slots. * Document how staff regain clearance via a migration PR Explain the staff clearance gate and, most importantly, how an affected staff member regains access: by submitting a migration PR that sets a verified name on the account. Records that a real name is PII and must be sourced from the deployment config rather than hard-coded in this public repo, and that production runs the migration on deploy so no database shell access is involved. * Share the clearance role set and correct the propagation timing Export the clearance role set (ClearanceRelevantRoles) and reuse it in the write-side guard instead of recomputing rolesSatisfying(KycGatedRoles), so the read and write sides share one definition. Correct the documentation: clearance propagates through two crons (the EVERY_MINUTE DB scan and the EVERY_30_SECONDS in-memory prime), so a change takes effect within about 90 seconds, not one minute.
github-actions
Bot
requested review from
TaprootFreak and
davidleomay
as code owners
August 1, 2026 12:39
…rification (#4565) * Pass the HTTP status of a signed response to its verifier A responseVerifier that rejects a response throws an error which replaces the original axios error, so the status those bytes arrived with is lost before any caller can log it. An unsigned 2xx and an unsigned 5xx then read identically, although one points at our own key or configuration and the other at an upstream outage. Hand the status to the verifier on both the success and the error path. The parameter is optional, so existing verifiers keep working unchanged. * Name the status and the faulty header when a Bank Frick response fails verification A failed signature check currently logs one of two fixed strings and nothing else, so an operator cannot tell an unsigned error page from an unsigned success, nor which of the two headers was at fault. During an upstream incident that is the difference between chasing our own key material and waiting for the bank. Append a bounded diagnostic tail: the HTTP status when it is a usable code, plus a per-header classification (missing, present, ok, unsupported). Header values are still never echoed - they are attacker-influenced bytes, and these messages reach logs, alert mail and persisted intent error fields. The existing message prefixes are unchanged, so log filters and alert rules that match on them keep matching. * Cover the unsigned-response case end to end for both a 5xx and a 2xx The two halves of the incident case were covered separately - the transport layer handing over a status, and the verifier turning it into a message - but never composed. Cover the actual failure through the real HttpService: an unsigned error page with no signature headers at all, asserted for a 5xx and for a 2xx, which produce identical bytes and headers and differ only in the status now carried into the message.
TaprootFreak
previously approved these changes
Aug 1, 2026
* Backfill staff verified names on PRD * Harden staff name backfill guarantees * Satisfy PostgreSQL migration syntax gate * Describe staff name as deployment variable
TaprootFreak
approved these changes
Aug 1, 2026
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.
Automatic Release PR
This PR was automatically created after changes were pushed to develop.
Commits: 1 new commit(s)
Checklist