Release: develop -> main - #4485
Merged
Merged
Conversation
…4482) * fix(auth): scope the login sign message per environment A wallet signature proves ownership of the exact text that was signed. That text was identical in every environment, so a signature stored in a lower environment verified on PRD as well. Non-PRD environments now prefix the sign message with their environment marker, which changes the signed payload and therefore the resulting signature. PRD keeps the historical text byte-for-byte, so existing PRD signatures stay valid. An unset or unknown ENVIRONMENT is deliberately treated as non-PRD: it must never accidentally yield a PRD-valid signature. Covered by two specs: one pinning the PRD text byte-for-byte and the fail-closed behaviour, one proving cryptographically that a signature made over the PRD text does not verify under the DEV text and vice versa. * feat(migration): rotate the DEV wallet signatures with an audited fingerprint The stored user.signature values on DEV were produced with the previous environment-independent sign message and would therefore still verify on PRD. This clears them. CONTRIBUTING.md requires an overwritten value to stay recoverable from the database. That cannot be satisfied literally here, and the conflict is inherent: the values are login credentials, so retaining them recoverably would preserve exactly what this rotation removes. Instead the migration writes an audit row to log before the update, holding the affected user id and an md5 fingerprint of the previous signature rather than its plaintext, and couples the update fail-closed to that insert via EXISTS (SELECT 1 FROM "audit") - following the precedent in 1784600000011-FixFiatOutputValutaDateSerials.js. A known candidate value can be checked against the fingerprint; the credential itself cannot be recovered. md5 acts purely as an audit fingerprint here. down() deliberately restores nothing: the prior value is not reconstructible by design. * test(auth): verify the signature rotation against a real database The migration spec only matched substrings against a mocked query runner, so an invalid CTE or a broken audit coupling would have passed unnoticed. Adds a MIGRATION_TEST_PG-gated Postgres block, following the pattern already used by most migration specs in this repo. Four scenarios: the rotation writes exactly one audit row whose fingerprints match independently computed md5 values; an empty candidate set writes no audit row and changes nothing; a failing audit insert (log table dropped) rejects and leaves the signature intact, which is the fail-closed property; and the environment gate holds against a real database. Also switches both sign-message specs to absolute imports as required by CONTRIBUTING.md, and states the reason for the require-import lint exception. * test(auth): isolate the audit coupling and pin the fingerprint-only guarantee The previous commit claimed its dropped-log-table test proved the fail-closed property. It did not: because the audit insert and the update are one statement, Postgres rolls the statement back on any insert error, so that test passes even without the EXISTS coupling. Verified by mutation - removing the coupling left it green. Its title now says what it actually shows, statement atomicity. The coupling is now isolated by a test where the insert SUCCEEDS but yields no row: a BEFORE INSERT trigger on log discards it without raising. up() then completes, no audit row exists, and the signature must stay intact. Removing the coupling turns exactly this test red. Also pins the property the audit exception rests on - no plaintext signature reaches the log, asserted negatively against both fixtures and against a before field - narrows the env-scope titles to the text difference they actually assert, since the cryptographic proof lives in the replay spec, and covers an unknown ENVIRONMENT value. * test(auth): make the down() assertion non-tautological The mock was created but never passed to down(), which takes no query runner, so the assertion observed an object the call could not reach. It would have stayed green through any regression. The unit test now asserts the actual property - down() takes zero parameters and resolves - and the behavioural guarantee is pinned against a live database instead: after up() followed by down(), the signature stays NULL and the single audit row remains. Passing the mock in would have meant giving the migration a parameter it does not need, changing production code to satisfy a test. * test(auth): assert the environment gate for every non-dev value Two titles claimed coverage for any non-dev ENVIRONMENT while their bodies set only 'prd'. A regression that executed on loc, on an unset value or on an unknown one would have left both green. Both are now table-driven over prd, loc, staging and unset - at the unit level and against a live database - so the fail-closed guarantee of the strict !== 'dev' check is pinned across the whole value space instead of being inferred from a single case.
github-actions
Bot
requested review from
TaprootFreak and
davidleomay
as code owners
July 30, 2026 08:16
…the completed files (#4483) * test(coverage): close the last uncovered branches on three files listKeys was never called without a prefix, the S3 WORM guard's non-Error fallbacks were never hit, and the scorechain screening entity's JSON getters were never read with a value set. Add the missing cases and pin all three files in the coverage ratchet. The unreachable `?? name` in the mock storage's getBlob is replaced by a slice: split always yields at least one element, so pop can never be undefined there and no test could ever cover that fallback. * test(coverage): pin four more completed files and refresh the numbers The gate run for the previous commit surfaced four files that already hold 100% on all four metrics but were never pinned; they arrived with recently merged PRs. Pin them and bring docs/coverage-gate.md to the measured state. Also records what the run made visible: part of the remaining branches are defensive fallbacks that cannot fire at runtime, so they are closed by removing the fallback, not by inventing a mock for them. * test(coverage): pin the last completed file and rebase the measurement Rebasing onto current develop moved the totals slightly and surfaced one more file already at 100%. Pin it and restate the numbers against the commit they were actually measured on. * docs(coverage): correct the Frick gate size and the importer counts The gate table still described the Frick gate as covering 7 files run by 7 specs; both the script and jest.frick.config.js have listed 10 for a while. The test-scaffolding note counted 60 and 28 importers, now 62 and 29 - all of them still specs, so the surrounding claim is unchanged. * docs(coverage): finish correcting the Frick gate size Two more places still said the Frick gate runs seven specs: the prose in the measurement section and the header comment of the ratchet config. A sweep over the three files confirms these were the last ones.
davidleomay
approved these changes
Jul 30, 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
2 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.
Automatic Release PR
This PR was automatically created after changes were pushed to develop.
Commits: 1 new commit(s)
Checklist