Repository navigation
Review of 1.28–1.33: RV1–RV8 - #69
Merged
Merged
Conversation
…o end plans/REVIEW_1_33_PLAN.md: findings on five axes (money path, security, features, interface, operations), every High and Medium traced to the code, and stages RV1–RV8 with the decisions each needs. Three Highs: a check paid after a decision never has its paid item or issued item marked paid, so a reverse positive pay account pays the same check twice by silence; bank deletion clears five of the seven storage areas, so decision files survive; a core's customer sync sets is_active unconditionally and reactivates an offboarded customer whose deletion stays scheduled. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… duplicate Until now only a clean match set a paid item's settlement and its issued item's status. A check decided pay (an accepted amount mismatch; every check on a reverse positive pay account, paid at the cut-off by silence) stayed pending, so presenting it again raised the same item again, and on a reverse account paid it again (REVIEW_1_33_PLAN.md T1, RV-D1). decision_service.lock now settles the check when the decision is final: paid or returned, and a paid check's outstanding issued item paid (a void is kept). Migration b8c4d2e6f1a3 brings decisions already final into line, per bank under RLS. Pay all posts the ids the page listed and pays exactly those (T11). A retried POST /checks/cashed is told which paid item the first call recorded (T10). Postgres tests: a branch cash against a file presentment, one verification cashed twice at once, Pay all against the cut-off sweep. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A decline for a payee the teller didn't send no longer carries the payee it asks for, and `issued` names the payee only when the one sent differs (T2). A check another teller verified in the last 15 minutes and hasn't recorded cashed is declined `being_cashed`, so two branches can't both pay it out; verifications of one check take turns on its issued item's lock, so the second sees the first (T5). The probing counter counts every decline, not only "not issued", by teller, branch, API client and, when a person's token asks, by user (a new teller_verification.user_id column, migration c9d5e3f7a2b4) (T3, T6). A check on an account whose customer's service has ended answers not_on_positive_pay instead of failing; an MRM replay reads such an account too (T4, O4). Also, found by RV1's suite run: backing out a file whose check was paid by a decision restores the issued check, as a matched one does, and a check already returned by a decision can't be backed out. The 1280px viewport baselines for four pages change because the fixture's decided checks are now settled. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…t-offs A check's decision deadline is capped at the cut-off on its return day. When a file reached OpenClear after that cut-off, the item was due at once and the customer's default applied at the next sweep, unseen (G1). It now gets the bank's minimum review time, never past the midnight deadline (RV-D7); one whose midnight has passed is still due at once. next_cutoff honours open_on_federal_holidays, as the setting's label implied (G6). The decisions CSV's columns are listed in the BI guide, with the column 1.29.0 added at the end (G2). The flagged-decision test asserts the CSV column and the webhook payload too. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ays when it fails _delete_files walked a hand-written list of five areas; storage has seven, so the decision files the core took and fraud-scoring artifacts survived a deletion whose certificate said "every file" (O1). It now walks storage.AREAS and the certificate lists them. The bank is frozen before the wipe, in its own transaction: inactive (every token check now refuses an inactive bank) and its job rows gone, so a worker mid-import loses its lease and nothing is added while the rows go (O7). A deletion the queue gives up on is marked failed, shown on the page and raised as a critical event (O9). Offboarding a bank, deleting its data and ending a customer's positive pay carry Kind.second_person: with nobody else able to approve, the request is refused instead of applied after the cooling-off (O3). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ywhere
Sign-in, the refusal of files naming its accounts and the teller answer
read the last day itself (ended(): service_ends_at <= now as well as
service_ended_at), not the daily run that follows it, and end_service
records the end as the end of that day, so the retention clock and the
pages agree with the day the customer was told (O5). PUT
/api/v1/customers/{number} with is_active: true is refused for an ended
customer: a core's sync can't undo the two-administrator decision (O2).
Kept people (named in the action log) are anonymised after their personal
rows go outside the savepoint (O6); the bank's attestations and access
reviews about the customer stay, unlinked, and the certificate says so
(O8); ending service ends sessions at this bank only (O11); dates on the
customer page, the banner and the Deleted customers list are the bank's
days (O10). _strip writes empty mappings into JSON columns, not null.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… holds per instance A negative number in a cf_ column of the decisions CSV reached the core as '-5, the formula guard's doing; a plain signed number now passes as one (C1). A customer's feed reader asking for a field not shown to the customer is told there's no such field, as for one that doesn't exist (C3). Restoring an archived field counts against the field and list limits (C4). An update file naming an unknown or archived field is refused whole, with the file's bank in hand at load (ImportSpec.load_takes_bank), and NA/null/None cells in such a file are text (C5, C6). Integer, date and decimal parsing match their messages (C7); a form reports every field's problem at once; a dropped choice option is retired only while a record holds it (C9, C10); field types read as words; one read for the account fields page. Rate limiting with Redis: while Redis can't be reached each instance limits on its own instead of allowing everything, the outage is one rate_limit.backend_down event, and the Controls page says so (rate_limit_backend check, control OC-SC-08) (C2). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…es the code A bank may name only its CEO and CIO, the fallback 12 CFR 304.24 gives; set_contacts required a named contact, so recipients()' fallback branch was unreachable while the page, the control and the CUEC described it (G3). The platform's Sent table says which banks each notice reached and how many addresses, marking a bank with nobody to email (G4); the form no longer offers "Offboarding", which offboarding sends itself (G5); the contacts form shows a row per contact and one more, up to ten (G7); the yearly confirmation is a security event (G8). A check refused at a branch is described in words on the customer's page (T9). The SAML signing-key page warns what finishing early or withdrawing late does (K3). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…uild and test pins openclear/errors.py::UserFacingError is the one type for sentences meant for people, and web/errors.py::shown() is how every router shows a refusal: the error's own text when OpenClear wrote it, a fixed sentence when a library or driver raised it, which is the class of finding CodeQL #176 raised, closed in all 117 places rather than one (K2). 37 service error classes inherit it; a test greps the routers for the old pattern. scripts/third_party.sh --check regenerates the third-party inventory and notices in the image's base and fails when the committed copies differ; the Security workflow runs it, so a stale license or notice text fails CI (K1). The bank's re-key of a customer's SAML connection goes through the customer's own second-person rule (K4). shows_number excludes hex runs (K6); postgres:16 is pinned by digest and a URL-scoped ZAP waiver needs an instance (K9); absurdly long numbers in a posted-check file fail as a row or a trailer problem, not an error (T8). Two 1280px baselines: the Controls page gained a control, and field types read as words. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The audit flagged pyasn1 0.6.4 in both locks; 0.6.5 fixes both. The third-party inventory and notices are regenerated for it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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
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
plans/REVIEW_1_33_PLAN.md: a review of what 1.28.0 to 1.33.0 added, and its eight fix stages, RV1–RV8.The three Highs
The rest
being_cashedwhen another teller has the check; the probing detector counts every decline; an offboarded customer's check answersnot_on_positive_payinstead of failing.cf_CSV columns, hidden keys refused for scoped readers, limits on restore, whole-file refusal of a badcf_header, strict parsing, per-field messages. The Redis rate limiter now holds per instance while Redis is down, with a security event and a control (OC-SC-08).web/errors.py::shown(): every router shows OpenClear's own sentence for a refusal and a fixed one for a library's exception text (CodeQL #176's class, closed in all 117 places);scripts/third_party.sh --checkin the Security workflow; pins and hostile-number guards.Migrations:
b8c4d2e6f1a3(settlement backfill),c9d5e3f7a2b4(teller_verification.user_id). API:being_cashedresult;issued.payee_nameoptional; 409 on/checks/cashedcarriespaid_item_id;PUT /customers/{n}refuses reactivating an ended customer; OpenAPI and SDKs regenerated.Security checklist
rate_limit.backend_down,incident_contact.confirmed,bank.delete.failed)80a8109c: SQLite 2329, Postgres 2337, Linux (see below)🤖 Generated with Claude Code