Repository navigation
security: a branch bound key never acts outside its branch - #83
Merged
Merged
Conversation
The sweep behind this test (PR body) found every route a machine key
reaches that writes or reads another branch's rows without the branch
middleware: the location module's unwalled writes (PUT and DELETE
/locations/{id}, POST /branches, PUT and DELETE /branches/{id}, the
user grant, revoke and home branch routes), the exposure surface's by
id routes on quotes, and the unwalled branch reads. ADR 0007 section
5.5 pins a bound key to its branch; only the branch middleware and the
payload guard honoured the pin, and these routes mount neither.
The tests drive serve's real mounts (branchWall.locations,
registerExposureRoutes) behind the real machine key core with a really
minted bound key, an unbound key and a user. All four fail at 1a719fd
on exactly the escapes: the bound key edits, archives, grants, revokes
and reads branch B's rows, writes an exposure event on branch B's
quote, and runs the dealer wide exposure scan.
Signed-off-by: colton <colton@futurebuild.ai>
…rywhere ADR 0007 section 5.5 pins a branch bound machine key to its branch, but only the branch middleware honoured the pin, and a machine key passes the role guards on its scope alone, so every route that mounted no branch middleware let a bound key act on any branch its body or path named: the location module's by id writes, the branch directory verbs, the user grant, revoke and home branch routes, the exposure surface's by id quote routes (whose actor fallback admits a key under AUTH_MODE=dev), and the dealer wide exposure scan. The wall (pkg/middleware/keybranch.go, beside the branch guard) reads the pin through branchctx.KeyBranchFromContext and compares the branch the request names by a path value, a body branch_id, or a row lookup (locations through their tree, quotes through their branch): a bound key naming another branch is refused 403 forbidden in the ADR envelope naming the field, with the key.branch_refused audit row, the same verdict the X-Branch-Id rule writes; its own branch passes, and a user and an unbound key pass unchanged, whatever the multi_branch_enabled switch says. The directory create and the scan refuse a bound key outright, being outside every pin. The branch users read of another branch is refused the same way, and a user's branch list is narrowed to the pin for a bound key. The wall mounts at the route registrations: the location handler takes it through WithKeyBranchWall (serve's branchWall builds it with the audit logger), and registerExposureRoutes composes it onto the exposure handler's guard. The two exposure lists and the branch directory reads stay un-narrowed: narrowing them changes recorded contracts (the exposure repository, PR 39's reference data decision) and the sweep lists them for the lead. Signed-off-by: colton <colton@futurebuild.ai>
CONTRACT-CHANGES gains the SEC-BOUND-KEY-PIN row stating exactly what changed on every route the key branch wall now covers. ADR 0007 5.5 gains the sentence that says how a route without the branch middleware is held to the pin and why the directory create and the dealer wide routes refuse a bound key outright. The drafts and locations module docs' bound key paragraphs now describe the fixed behaviour: the gap text (an operator should not mint a branch bound key with users:grants, branches:write or locations:write) is gone, replaced by what each route answers, with the branch directory reads, the exposure lists and the events feed named as the stated limits the sweep listed for the lead. Signed-off-by: colton <colton@futurebuild.ai>
The promotion test flips multi_branch_enabled globally with no lock, and a package whose fixture reads the setting mid flip (CI: the invoice credit memo test's sales caller) caches the flipped state for its middleware's 30 second TTL and answers 400 X-Branch-Id header required where its role guard owns the 403. Every other settings flipper (the serve wall and key pin fixtures) already rides the outbox lock; this one now does too. Signed-off-by: colton <colton@futurebuild.ai>
…ound-key-pin Signed-off-by: colton <colton@futurebuild.ai>
…er, red first The second review proved the wall and the handler read the body differently: the wall's json.Unmarshal fails on a body with trailing data and so names no branch, while the grant and home branch handlers' json.Decoder reads the first value and ignores the rest, so a key bound to A wrote a grant for B and moved a home branch to B with no audit row. Five bodies from the review, red at 7ef2d1d: all five land the write with 204. Signed-off-by: colton <colton@futurebuild.ai>
BodyBranch parsed with json.Unmarshal and let an unparsable body through as naming no branch, while the grant and home branch handlers decode with json.Decoder and read only the first value: a valid object followed by trailing data passed the wall and the handler acted on the first value. The wall now decodes into the same branch_id uuid.UUID the handlers decode, and a body that is not exactly one complete JSON value is refused for a bound key with the same 403 and key.branch_refused row as a foreign branch, so the wall and the handler can never disagree about what a body names. A body that parses and names no branch (absent, null) still passes to the handler's own 400, and the explicit nil UUID now meets the handler's branch_id required check rather than a wall refusal. Users and unbound keys never reach the parse. Signed-off-by: colton <colton@futurebuild.ai>
The sweep the reviews ran past the wall: the reporting module in all three of its registrations, the events feed, the two exposure lists, the GL, AP and bank reconciliation reads, the sales team reads, the known users list and the market index refresh are all open to a branch bound key today (30 routes, each one landing its module answer instead of the 403), while the unbound key and the user already answer the same as each other on every route. Signed-off-by: colton <colton@futurebuild.ai>
The lead's ruling on the reads and writes the sweep missed (TO-PRINCIPAL 51): every reporting route in its three registrations, the events feed, the two exposure lists, the GL, AP and bank reconciliation reads, the sales team reads, the known users list and the market index refresh now refuse a branch bound key outright with the key.branch_refused row. The GL, AP and bank reconciliation writes carry no branch fact and keep the role guard alone for every principal, as does the index metadata and history beside the refresh. Users and unbound keys keep exactly their reach: the test walks all thirty routes with the unbound key and the user and they answer the same as each other, 200 on the reads that answer data. Signed-off-by: colton <colton@futurebuild.ai>
The lead's ruling on the customer scoped writes: the tax exemption writes and the customer priced rules (plain and category, single and bulk) are open to a branch bound key today, so a key pinned to A writes branch B's customers' data: eight write families land their module answer instead of the 403, with the rows written. The unbound key and the user answer the same as each other on every route, each against its own fresh target. Signed-off-by: colton <colton@futurebuild.ai>
…the customer's branches The lead's ruling on the customer scoped writes: the tax exemption writes and the customer priced rules (plain and category, single and bulk) now reach a branch bound key only while every customer the request names, or that the row it acts on belongs to, holds the pin among the customer's branches (customer_branches), the same visibility the customer module holds a caller's branch context to; anything else is the 403 refusal with its key.branch_refused row, the details naming customer_id. A request that names no customer passes (it writes dealer wide data), a body that is not exactly one complete JSON value is refused like the body branch wall refuses it, and a lookup failure answers 500 rather than failing open. The reads keep the role guard alone; users and unbound keys never reach the wall. Signed-off-by: colton <colton@futurebuild.ai>
…ncluded pkg/middleware had no test file for keybranch.go at all, and the first review's mutant M6 (a branch lookup error fails open) survived every test. The unit tests walk every constructor with a recording auditor and no database: a pin and a foreign branch by path, body (whole, or refused), row and customer, the unparsable and trailing data bodies, the malformed and missing ids that pass to the handler's own answers, the no pin pass through, and the failure paths that must never reach the handler: a lookup error and a nil database answer 500. Applied alone, M6 now dies on the lookup error case (the handler ran, want the wall to fail closed). Signed-off-by: colton <colton@futurebuild.ai>
The second review seeded a legacy location row with a NULL branch (user triggers lifted) and the wall passed it: the bound key PUT and DELETE answered 200 and 204. ErrRowNamesNoBranch now marks a row that exists but belongs to no branch, and RowBranch refuses a bound key it like a foreign branch, red first. The switch off case pins the wall's independence from the multi_branch_enabled switch: with the switch off the pin still refuses the foreign branch and passes its own, and the unbound key and the user keep their reach. Signed-off-by: colton <colton@futurebuild.ai>
ADR 0007 section 5.5 now states the wall's whole final behaviour: the whole body read, the outright refusals (the directory create, the exposure scan and lists, the market index refresh, the events feed, every reporting route, the dealer wide GL, AP and bank reconciliation reads, the sales team reads, the known users list), the customer_branches confinement of the customer scoped writes, and the branch directory reads limit in one sentence with the PR 39 reference data reason. drafts.md and locations.md say the same for their routes, locations.md adds the home to own branch side effect on the user's other grant rows, the CONTRACT-CHANGES row lists every route and the 403 for a missing branch id where the base answers 404, and its stray trailing blank line is gone. Signed-off-by: colton <colton@futurebuild.ai>
…ed first The pin fixture now drives the production chain (buildChain) instead of a hand built machine key mount, mounts the staff roster, seeds a job per customer, a tier category rule and a certified tax exemption, and the round 4 findings become assertions: the refresh preview, the GL, AP and bank reconciliation writes, the staff roster, the unfiltered pricing lists and the matrix refuse a bound key; the customer scoped reads (the exemption read by path customer, the category rule list by query customer, the rule audit, calculate, resolve, the tax preview by body customer) are confined; a pricing rule naming a job is confined through the job's customer; a rule naming neither a customer nor a job, and a tier category rule on every verb, are refused; the stated limits (the branch directory reads, the index metadata and history) stay open; and a path no route resolves is refused. The GL writes block that held them open is gone: the ruling changed. Four of the five new tests fail on the base head, the stated limits test pins the open behaviour. Signed-off-by: colton <colton@futurebuild.ai>
One table, core/pkg/middleware/keybranch_allow.go: every route a machine key can reach (345 of them, method plus pattern exactly as the census names them) carries exactly one class. WALLED (183) runs behind the branch middleware; CONFINED (28) is held by the key branch wall or the customer branch wall, each entry naming its test; STATED LIMIT (69) is reachable unconfined by design, each entry with its one line reason; REFUSED (65) answers a bound key 403 forbidden with the key.branch_refused row. The check runs once: KeyBranchRouteGate, mounted in buildChain inside auth (it reads the pin the machine key core sets) and outside the confirm gate and idempotency (a refused request claims nothing). It resolves the request's registered pattern through the serve mux and refuses a bound key every route the table does not admit, including a route the router resolves to no pattern: a missed mount can no longer fail open, whatever the wiring beneath the chain does. A request with no pin passes untouched. The per mount refusals the opt in approach added are gone (the gate is the one place refusals live): the reporting, events feed, GL, AP, bank reconciliation and sales team mounts return to their role guards, the index admin guard loses its suffix matching (which missed the refresh preview), the exposure wall keeps only its by id confinement, and the location handler drops the directory create and known users walls. RefuseBound leaves the wall's API with its test. The census test fails when a machine key reachable census route is unclassified, when an entry matches no census route, and when a confined or stated limit entry carries no justification; it runs in the untagged suite with no database. TestKeyBranchPin_AllowlistGate- CoversCensus drives every reachable census route through the production chain with stub handlers: the bound key is refused exactly the 65 REFUSED entries, and the unbound key passes all 345. The pin fixture now builds its chain with buildChain, the builder Run uses. Signed-off-by: colton <colton@futurebuild.ai>
The customer branch wall now resolves every field that names a customer scoped row, not only customer_id: - a pricing rule's job_id resolves to its projects row's customer, and a body naming a customer and a foreign job is refused (both are checked), closing round 4 P1: a bound key can no longer set the price a branch B customer pays on a branch B job with no audit row; - a rule that names neither a customer nor a job is a dealer wide write and is refused (round 4 P2), as is a category rule or bulk element that names no customer (a tier target prices every branch), through CustomerBodyBranchRequired, which refuses an object or element with no customer_id; - the customer scoped reads are confined like the writes (round 3 P1-2, round 4 P2): the exemption read by its path customer, the category rule list by its query customer (absent is the unfiltered list and is refused), the rule audit by its row's customer, calculate and resolve by their query customer, and the tax preview by its body customer when it names one. A category row that targets a tier names no customer and is refused on every verb, and a bulk delete naming a tier row is refused whole. The walls dispatch on r.Pattern, the registered pattern the mux sets, not on path suffixes or prefixes. Signed-off-by: colton <colton@futurebuild.ai>
Round 3 P3-1, both arms now pinned: the location row lookup's SQL error arm fails closed (a cancelled query answers 500 with the handler never reached; the mutant that swallows the error into no branch named makes the test fail), and a bulk category delete body that is not exactly one complete JSON value is refused for a bound key (the decode error mutant makes the test fail). Both mutants were green on the previous head. Round 3 P2-2, the source half: the behaviour tests drive the fixture's mounts, so Run() dropping a confinement call would leave them green while production lost its wall. TestServeWiringMountsTheConfinements pins serve.go's five confinement call sites and chain.go's gate mount; the behaviour half (the fixture builds its chain with buildChain, and TestKeyBranchPin_AllowlistGateCoversCensus drives the gate over every reachable census route) makes a dropped gate fail red on its own. Signed-off-by: colton <colton@futurebuild.ai>
ADR 0007 section 5.5 now states the deny by default rule itself: the four classes, the one gate, the census test, the completed confinement contract (jobs resolve to their projects row customer, the customer scoped reads are confined, dealer wide writes are refused), the stated limits each with their reason, the do not mint advice round 3 P3-2 asked for, and the refused surfaces (the GL, AP and bank reconciliation writes included). The SEC-BOUND-KEY-PIN row, the drafts module page and the locations module page say the same shape. Signed-off-by: colton <colton@futurebuild.ai>
Signed-off-by: colton <colton@futurebuild.ai>
…ound-key-pin Signed-off-by: colton <colton@futurebuild.ai>
…e key branch wall Round 6 P1-1: a tier category rule that also named a customer passed the customer body wall (it reads customer_id only), was written, and priced every branch's customers of that tier while the allowlist said tier targets are refused. The wall's category body resolver now decodes the same target the handler parses (target_type, tier, customer_id, per element of a bulk body) and refuses any element whose target is not an account rule or that carries a tier; the handler keeps the invariant on its own side (a tier rule carries no customer, an account rule no tier); migration 107 puts the invariant on the table, checking existing rows first: a row that breaks it is reported by a NOTICE naming the count and ids, never rewritten, and the constraint goes on NOT VALID when any exists, validated on add otherwise. Signed-off-by: colton <colton@futurebuild.ai>
Round 5 P1 and round 6 P1-2: the till, Z report, return and account routes loaded their row by id with no branch predicate, so a key bound to A closed B's till (posting its over/short and freezing its Z report), read B's till report, Z reports, returns, account summary, AR statement and reconciliation, and a line insert on B's transaction left the line written behind its 500. The till, Z report and return reads now carry the branch predicate the transaction reads already carried (till_sessions.branch_id, till_z_reports.branch_id, pos_returns.branch_id); the line delete joins its transaction's branch; AddItem reads the transaction (through the same predicate) before anything is written, so the insert never happens on a transaction the caller may not target; the account summary, subledger and statement mount the customer branch wall on the path customer; and the reconciliation, whose ledger half carries no branch fact, is refused to a bound key by the route allowlist. The pos handler file is untouched (PR 72 rewrites it): the confinement lives in the repository queries and the branch wall layer. Signed-off-by: colton <colton@futurebuild.ai>
…tomer Round 5 F-3: GET /pricing/calculate confined only its customer_id, so the pin's customer plus the foreign customer's job answered with the job override rule that prices the foreign customer, byte for byte the unbound key's answer. The calculation's resolver now resolves job_id to projects.customer_id, the same resolution the rule write carries, and holds both customers to the pin; a job that names no customer changes nothing. Signed-off-by: colton <colton@futurebuild.ai>
…s customer Round 6 P3: POST /tax/preview with the foreign branch's ship_to_id and no customer passed the wall (the resolver read customer_id only) and answered a rate for a delivery the key's branch does not serve. The preview's resolver now resolves ship_to_id through customer_ship_tos.customer_id and holds that customer to the pin beside the body's customer_id; a preview naming neither stays a plain rate calculation. Signed-off-by: colton <colton@futurebuild.ai>
…ion path probe
Round 5 P3: the gate fails closed when its mux is not a pattern resolver
(mutant G5 survived every test; the pin drives a plain handler mux and
expects the refusal, and goes red under the mutant); the twenty REFUSED
routes no independent test named (staff id and modules, reporting saved and
schedules, AP invoice read and approve, bankrecon session read, account
create, complete and unmatch, GL reopen, post, reverse, void and account
write) join DealerWideReads by name with their parity legs; and
PUT /locations/{id} writing path is probed and pinned: the path is a
materialized breadcrumb with no enforcement, the branch comes from the
parent at insert (the denorm trigger fires on parent_id and type, neither
writable by the route), and no branch scoped read resolves a branch from a
path, so a path naming the foreign branch's breadcrumb cannot move a row
under it.
Signed-off-by: colton <colton@futurebuild.ai>
…er writes it The principal's ruling (FROM-PRINCIPAL 37 and 38): every STATED LIMIT write is refused to a branch bound key, the reads stay stated limits; the dealer wide catalogue writes the branch middleware cannot confine (products, PIM content, margins, dimensions, lead time, kit components, unit sets, payment terms, charge codes, the reorder target refresh) are dealer wide writes and are refused with their reads stated; and the driver routes are refused outright, reads included (PD-3: driver rows carry personal data and no branch column), while the vehicle reads stay stated limits. The class split in the table: WALLED 145, CONFINED 31, STATED LIMIT 51, REFUSED 118 of the 345 machine key reachable routes. One A/B leg per write family drives every refused family with its audit row beside the unbound key's own module answer, and the margin write proves the refusal is the only thing that kept the dealer wide target margin unwritten. Signed-off-by: colton <colton@futurebuild.ai>
… a twin sweep The lead's ruling on item 37: WALLED becomes a tested claim. Every WALLED entry's note now names the column or join its module filters on (quotes, orders, invoices, payments, credit memos, purchase orders, drafts and till rows on their branch_id; customers, contacts, ship tos and activities through customer_branches; deliveries and routes through their orders' branch; the dashboard and inventory through locations.branch_id; the links through each module's walled read), replacing the bare the branch middleware label that rounds 5 and 6 showed can overstate. A new sweep drives every WALLED route that takes an id or a list with branch B's twin row as a bound A key's target: the foreign row answers exactly what a missing row answers (or a 403), never names the row, nothing is written (family tables counted before and after), a user reads the twin where the missing row is a 404 so the twins are real, and the lists carry the pin's branch only. The fixture grows the document family mounts exactly as serve registers them and a second mint pair for the document scopes (the mint caps a key at 64 scopes). Signed-off-by: colton <colton@futurebuild.ai>
…mers, audited grants and AI spend PD-4, the stricter line: a branch bound key may read a customer shared with another branch, but may not add or change a customer scoped pricing rule, category rule or tax exemption for it, because the write prices the other branch's sales to that customer too. A new exclusive customer wall holds those writes to the customers the pin holds alone and refuses the rest with a message naming the other branch; the reads keep the shared customer rule. PD-5: the grant, revoke and home branch writes for the pin branch record their audit rows (user.branch.granted, user.branch.revoked, user.home_branch.set) with the machine key named as the actor. PD-6: the parsing upload and the vision scan stay stated limits and each call records its ai.parsing.upload or ai.vision.scan audit row with the key that spent the dealer's AI credit (both handlers gain the audit logger serve wires). The drivers ruling (PD-3) landed with the stated limit split. Signed-off-by: colton <colton@futurebuild.ai>
…e code now does The ADR's section 5.5 records the principal's rulings: WALLED is a tested claim whose entries name their filter column; the category rule invariant and its migration; the stricter shared customer line (read yes, priced writes no, the refusal naming the other branch); the audited grant, revoke and home branch writes; the stated limit as a read, never a write (the catalogue and configuration writes, the drivers refused reads included, the AI spend audited); and the account and calculation confinements. CONTRACT-CHANGES gains the fix round 3 row naming every changed answer, and the delivery, charge codes, products and locations module pages state the same: the fleet split, the refused catalogue writes, the audited grants. Signed-off-by: colton <colton@futurebuild.ai>
…ound-key-pin Signed-off-by: colton <colton@futurebuild.ai>
…PR 83 r13 P2-1) CreateTransaction, CreateTillSession and GetRegisterBranch derived the register's branch only through locations, so a register whose location names no branch skipped the pin check and wrote at the default branch. COALESCE(l.branch_id, r.branch_id) makes the NOT NULL column the fallback. Tests per register branch derivation plus r13's reproducing pos sync test; the allowlist notes now say what the code reads. Signed-off-by: colton <colton@futurebuild.ai>
…nd key's words (PR 83 r13 P3-1) A user without the finance or owner role and an unbound key without customers:restricted were refused with different words from the bound key's and left no audit row. They now carry the bound key's words, and a customer.shared_restricted_refused row names the actor, the fields, the lowest numbered holding branch and the method and path. Signed-off-by: colton <colton@futurebuild.ai>
…y scoped bound key (PR 83 r12 P3s) Each test goes red when its guard is removed: the conversion's stored job recheck, ownerGrantsRestricted with no claims, and a bound key that carries customers:restricted on a shared customer. Signed-off-by: colton <colton@futurebuild.ai>
…PR 83, item 54 question 1) The customer group admitted finance on every route. A third role guard now admits finance on the header write, the salesperson and the escalation policy only; customer and contact create, the reads and the other writes answer finance what refactor/v1 answered. No ADR or role catalog row grants finance more. Signed-off-by: colton <colton@futurebuild.ai>
… 83, item 54 question 2) An activity has no branch tag, so on a customer another branch also holds a bound key adds one and never changes or deletes one: 403 in the contact rule's words with a key.branch_refused row, the activity untouched. The crm service takes the shared guard through the customer guard adapter. Signed-off-by: colton <colton@futurebuild.ai>
…and the module pages state fix round 6 Signed-off-by: colton <colton@futurebuild.ai>
…EG-01 onto Kelowna The till session opened on a register with no branched location now carries the register's branch, so branch_id appears in its body (the four pos goldens, ids renumbered). The seed moves REG-01 onto Kelowna so a seeded register belongs to the default branch the seed makes, which leaves every sale and return amount unchanged. Listed in CONTRACT-CHANGES. Signed-off-by: colton <colton@futurebuild.ai>
Signed-off-by: colton <colton@futurebuild.ai>
Signed-off-by: colton <colton@futurebuild.ai>
…return rows (r15 P2-1) The till session, Z report and return reads filter on their branch_id, so every row written before the column was stamped (a register whose location names no branch, which includes the seeded REG-01) vanished from every caller with a branch context: an open drawer left at upgrade read as no drawer and could be neither closed nor replaced, and the legacy Z reports and returns disappeared from the branch scoped reads. Migration 108 backfills the three tables from each row's own register (the round 6 rule, COALESCE(the register's location's branch, pos_registers.branch_id)). It touches only rows whose branch_id IS NULL, sets nothing else, moves no money and deletes nothing, so it is idempotent. A row whose register names no branch is left exactly as it is and reported by a NOTICE naming the count and the first ids. The down file leaves the data: the branch was read from the row's own register and nulling it again would put the rows back outside the branch scoped reads. Tests: the migration runs in a scratch database seeded with legacy rows on every table it touches (a locationless register, a register on a yard whose branch beats its own, rows already set, rows whose register was never there), asserting the NOTICE counts, the resolved branches, the untouched rows and money digests, and a second run that changes nothing; the serve legs plant the legacy rows, apply the migration and read the drawer, its Z reports and the returns back through the real chain with the register's branch selected. Signed-off-by: colton <colton@futurebuild.ai>
…0 (r15 P3-1) The transaction create refuses a request whose branch context disagrees with the register's own branch (the round 6 rule), but the handler mapped every service error to 500, so the refusal read as an internal error. CreateTransaction now returns a typed RegisterBranchRefusedError and StartTransaction answers it 403 with the refusal's words, the same envelope the machine key cashier refusal uses (httputil.RespondError answers the generic message of the status). Nothing is written either way; the offline sync keeps reporting the same refusal per item in its errors array. Test: an admin with a branch selected selling on another branch's register answers 403 naming the rule with no transaction written, and the control sale on the caller's own register still answers 201 at its branch. Signed-off-by: colton <colton@futurebuild.ai>
…ster's (r15 P3-2) A till open and a return stamped the register's branch whatever the caller's branch context named, so a cashier granted only one branch, with that branch selected, opened a till and recorded a cash return on another branch's register. The register on a branched location half of that predates this PR; the locationless register half came with round 6's branch fallback. Both now refuse the way the sale already does, before anything is written: CreateTillSession and ReturnSale return the typed register branch refusal when the request's branch context is set and differs from the register's own branch, and the handlers answer it 403 with the refusal's words. The caller's own register passes for both. Test: an admin with branch A selected on a register of branch B answers 403 for the till open and the return with no session and no return written, and the A controls answer 201 with the session and the return at branch A. Signed-off-by: colton <colton@futurebuild.ai>
…re the log row (r15 P3-3) The sync logged its batch under the batch register_id whatever the pin held, so a key pinned to A sending batch register regB with an item on regA wrote a pos_sync_log row naming another branch's register. The batch register is the register the client says the batch ran on, so it is held to the caller's branch context the way each replayed create is: the whole batch answers the typed register branch refusal (403) before any item runs and before the log row is written. The walled route sweep's sync leg becomes a held leg (the module itself now refuses the foreign batch register) and its control on the pin's own register still reaches the module. Also resolves the duplicate CONTRACT-CHANGES id: the pos row of round 6 becomes SEC-BOUND-KEY-PIN-7, the id this round's rows carry; the allowlist notes state the batch register hold and the 403 register refusals on the sale, the till open and the return; ADR 0007 section 5.5 states the sync's batch register rule. Test: a bound key's sync with another branch's batch register answers 403 with no pos_sync_log row and no replayed transaction, and the control batch on the pin's own register syncs and logs its one row. Signed-off-by: colton <colton@futurebuild.ai>
…tem 57 question 1) Finance passes only the three restricted field routes but was refused the two reads those writes need, so it could never hold the revision it must send as If-Match on a record it cannot read (r14 section 3, the principal's item 57 ruling: the two reads open, reads only). The record read and the escalation policy read move to the third guard (the restricted field guard), so finance reaches exactly the three restricted writes and the two reads they need; every other customer, contact, ship-to and activity route keeps the roles it had. The round 6 test reads its revisions as finance now, and asserts finance's two reads answer 200 while every other route of the groups still answers the 403 insufficient role. Signed-off-by: colton <colton@futurebuild.ai>
…ause (item 57 question 2) Every shared customer refusal served the bound key's words, so a sales user read "a branch bound key may act only on its own branch", which is wrong for a user (the r14 and r15 reviews' note, the principal's item 57 ruling). The refusals now carry one clause naming the other branch behind a lead in that names the caller, words true for whoever reads them: the bound key's lead says "this key may act only on its own branch", an unbound key without the customers:restricted scope reads "this key may not change a shared customer's restricted fields", and a user without the finance or owner role reads "this role may not change a shared customer's restricted fields". The verdict marks a caller refusal served to a machine key so the service can choose the key's or the role's lead; the child rule (ship-tos, contacts, activities) refuses bound keys only and keeps the key's lead. The key branch wall's own refusals keep their words (they are served to a bound key alone). ADR 0007 section 5.5 quotes all three. Tests assert each caller's exact words: the round 7 leg drives the bound key, the unbound key and the sales user; the round 5 and round 6 legs are updated to the new words. Signed-off-by: colton <colton@futurebuild.ai>
…ds it owns (item 57 question 3) The seed moved REG-01 onto Kelowna with ON CONFLICT (id) DO UPDATE SET location_id, branch_id, name, resetting a register an operator had renamed on every re-seed (r14 P3-2), and the principal's item 57 condition admits the upsert only while it can never move a register that a person, an import or another seed created. Kept the upsert rather than going back to DO NOTHING, because with DO NOTHING a migrated database holds REG-01 on the nil location and the migration-time default branch, which on a seeded database is not Kelowna, so the demo register's sales would resolve to the wrong branch and the goldens' branches and taxes would change. The conflict stays on the id, so only the migration's own demo register row can ever be touched: a register with any other id, created by a person, an import or another seed, is never matched. Of that one row the update now sets the location and the branch alone, the fields the seed owns; the register's name is kept. The statement becomes the package level posRegisterUpsertSQL so the test runs the seeder's actual upsert twice: a register created outside the seed is left exactly as it was, and the seed's own row moves idempotently while keeping an operator's rename. Signed-off-by: colton <colton@futurebuild.ai>
CONTRACT-CHANGES gains the fix round 7 row (SEC-BOUND-KEY-PIN-7: the backfill migration, the 403 register refusals on the sale, the till open, the return and the sync batch, finance's two reads, the caller specific refusal words), and the pos row states the seed's REG-01 upsert as it now stands: on the id alone, setting the location and the branch only, keeping a register's name. The customers module page states finance's two reads and the three caller specific refusal lead ins; the crm activities page quotes the activity refusal's new words. Signed-off-by: colton <colton@futurebuild.ai>
Migration 027 creates REG-01 on every database, so the row is not the seed's own however the desk hardcodes it, and the unconditional DO UPDATE moved a REG-01 an operator re-pointed back onto Kelowna. The update now fires only while the row still carries 027's placeholder location, so a re-pointed register stays where its operator put it (the r16 P2 reading of item 57's condition), a second seed run is a no-op, and the name was never touched either way. The test forces the row's migration shape first, so it runs on any database. Signed-off-by: colton <colton@futurebuild.ai>
The report handler mapped every service error to 500, so a session the caller's branch context hides answered 500 where the other by id reads (the Z report, the return) answer 404. A hidden session now reads as missing the same way, nothing leaked either way (r17 P3-1). The spec entry, the allowlist note and the pos_till_close golden follow the new answer. Signed-off-by: colton <colton@futurebuild.ai>
The wall that holds a customer scoped pricing or tax write on a shared customer still led its refusal with "a branch bound key may act only on its own branch", words ADR 0007 section 5.5 does not quote; the customer service's round 7 refusal already carries "this key may act only on its own branch". The wall now carries the same quoted lead in, so every shared customer refusal a bound key meets carries words the section quotes. The ADR and the customers page say so. Signed-off-by: colton <colton@futurebuild.ai>
A sync batch naming a register that does not exist ran its items and was logged under that register, because the batch check ran only when the register resolved (r17 P3-3). The batch is now refused 400, a typed error naming the register, before any item runs and before the pos_sync_log row, and a lookup failure answers an error rather than skipping the hold. The spec entry, the allowlist note and the ADR's sync passage follow. Signed-off-by: colton <colton@futurebuild.ai>
Every row of this PR now carries a unique id (r16 P3): the pos row (the till session wire change and its goldens) its own SEC-BOUND-KEY-PIN-POS, each round's rows under their own id, and round 8's row SEC-BOUND-KEY-PIN-8 records this round's findings (round 7 had left the pos row and its own row both on SEC-BOUND-KEY-PIN-7). Signed-off-by: colton <colton@futurebuild.ai>
Signed-off-by: colton <colton@futurebuild.ai> # Conflicts: # docs/refactor/CONTRACT-CHANGES.md
…ound-key-pin Signed-off-by: colton <colton@futurebuild.ai>
Round 8's guard keyed on the placeholder location alone. The placeholder names no locations row, so while REG-01 sits on it the register's branch comes from pos_registers.branch_id alone on every pos read and write, and an operator who re-pointed the register by its branch alone had moved it as completely as one who moved the location; the next demo re-seed moved it back (r19 P2-1, item 59's the operator's row is never moved). The upsert now also requires the branch migration 066 gave the row, the default_branch_id the seed reads before it changes anything, so it fires only on the row exactly as migrations 027 and 066 left it. A re-seed leaves a branch only, location only or both re-point alone, still moves a row in the migrations' shape exactly once, keeps every name, and a database whose default an operator changed keeps REG-01 where it is. Proven with the seed's own statement (red first) and with the real cmd/seed over two runs per leg. Signed-off-by: colton <colton@futurebuild.ai>
The unknown batch register refusal ran only when the register_id was not empty, so a batch with an empty register_id still ran its items and was logged under a register nobody made, the log row carrying an empty register_id (r19 P3-1). An empty register_id names no register either, so the batch is now refused the same typed 400 before any item runs and before the pos_sync_log row, for every caller: a register that does not exist is a bad request for anyone (r18 P3-2's confirmation, stated in CONTRACT-CHANGES as SEC-BOUND-KEY-PIN-9). The spec entry, the allowlist note and the ADR's sync passage follow. Signed-off-by: colton <colton@futurebuild.ai>
The exclusive walls leg counted the fixture's bound key rows, never the pinned key it drives, and its two later checks were placeholders that could not fail, so the leg that carries the wall's quoted refusal words never checked that the refusal wrote its row (r19 P3-2). The test now reads the pinned key's id after the mint, asserts its key.branch_refused count rose by exactly one, and reads the row back: the refused call's method and path and the pin branch. The module page states the row. Signed-off-by: colton <colton@futurebuild.ai>
The row states the three round 9 fixes and the round 18 confirmation: the seed's upsert fires only on the exact shape migrations 027 and 066 left, an empty or unknown sync batch register is a 400 for every caller, and the exclusive wall test asserts the refusal's audit row. It also records that the ids repeating on refactor/v1 (PR 99's C4-1b rows among them) predate this PR and are left alone: a whole file pass on citable ids is the principal's hygiene work. Every id of this PR stays unique. Signed-off-by: colton <colton@futurebuild.ai>
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.
What
A branch bound machine key never acts outside its branch, denied by default. ADR 0007 section 5.5 pins a bound key to its branch; the opt in fix that walled each route the sweep found failed open on every route a round missed (rounds 3 and 4 kept finding siblings: the index refresh preview, the customer scoped pricing and tax reads, a pricing rule naming a branch B customer's job, the dealer wide pricing writes, the GL, AP and bankrecon writes). This round replaces it: one allowlist (
core/pkg/middleware/keybranch_allow.go) classifies every route a machine key can reach (345 census routes, method plus pattern), and one runtime check,KeyBranchRouteGatemounted once in serve's chain inside auth, refuses a bound key every route the table does not admit, a route the router resolves to no pattern included. A census test in the untagged suite fails when a machine key reachable census route is unclassified or a table entry is stale. A user and an unbound key are unchanged everywhere, whatever themulti_branch_enabledswitch says.The sweep
Every route a machine key can reach that does not run behind the branch middleware, and writes (or reads another branch's rows):
Writes with no branch fact (no escape, unchanged): units, parsing upload, pricing categories, category rules, rebates, market indices, vendors, gl, edi, ap, bankrecon, reporting, techadmin settings, staff and module switches, apps, vision, millwork, configurator, governance. Not reachable by a machine key at all:
/api/portal/*,/api/integration/*,/api/partner/*(the partner segment is not in the scope vocabulary),/api/v1/a2a,/api/v1/admin/keysand/api/v1/me(user only). Flagged for the lead and not fixed here: the tax exemption routes name a customer whose branch set is many valued (customer_branches), so refusing needs a policy for a customer that spans the pin's branch and another.Each refusal with its test
Every refused route is driven through serve's real mount methods (
branchWall.locations,registerExposureRoutes) behind the real machine key core with a really minted key, incore/internal/app/serve/wire_key_branch_pin_test.go. Each matrix is: bound to A against B (403forbidden, one freshkey.branch_refusedaudit row, no write), against A (allowed), unbound key and user (unchanged). All four tests were red first at 1a719fd (22 assertion failures, every one an escape, including a real cross branch exposure event write).TestKeyBranchPin_LocationWrites: PUT and DELETE/locations/{id}, POST/branches, PUT and DELETE/branches/{id}.TestKeyBranchPin_UserGrants: the grant, revoke and home branch routes, by body and by path.TestKeyBranchPin_LocationReads:GET /branches/{id}/usersrefused for another branch;GET /users/{sub}/branchesnarrowed to the pin for a bound key.TestKeyBranchPin_ExposureRoutes: the five by id exposure routes (foreign quote refused, own quote passes to the module: 200, 200, 200 with the event written, 409, 409), the scan refused for a bound key, unbound key and user unchanged, and the auth core's own X-Branch-Id rule standing in front.Docs
CONTRACT-CHANGES gains the
SEC-BOUND-KEY-PINrow stating exactly what changed. ADR 0007 section 5.5 gains one paragraph saying how a route without the branch middleware is held to the pin and why the directory create and the dealer wide routes refuse a bound key outright.docs/modules/drafts.mdanddocs/modules/locations.mdbound key paragraphs now describe the fixed behaviour, with the branch directory reads, the exposure lists and the events feed named as stated limits.🤖 Generated with Claude Code
Fix round 2: the allowlist (deny by default)
Class counts over the 345 machine key reachable census routes after fix
round 3 (the round 2 counts were WALLED 183, CONFINED 28, STATED LIMIT 69,
REFUSED 65):
key.branch_refusedaudit row)Every CONFINED entry:
DELETE /api/v1/branches/{id}DELETE /api/v1/locations/{id}DELETE /api/v1/pricing/category-rules/bulkDELETE /api/v1/pricing/category-rules/{id}DELETE /api/v1/tax/exemptions/{id}DELETE /api/v1/users/{sub}/branches/{branch_id}GET /api/v1/branches/{id}/usersGET /api/v1/pricing/calculateGET /api/v1/pricing/category-rulesGET /api/v1/pricing/category-rules/{id}/auditGET /api/v1/pricing/resolveGET /api/v1/quotes/{id}/exposureGET /api/v1/tax/exemptions/{customerID}GET /api/v1/users/{sub}/branchesPOST /api/v1/pricing/category-rulesPOST /api/v1/pricing/category-rules/bulkPOST /api/v1/pricing/rulesPOST /api/v1/quotes/{id}/exposure/acknowledgePOST /api/v1/quotes/{id}/exposure/escalate-nowPOST /api/v1/quotes/{id}/exposure/overridePOST /api/v1/quotes/{id}/exposure/request-ackPOST /api/v1/tax/exemptionsPOST /api/v1/tax/previewPOST /api/v1/users/{sub}/branchesPUT /api/v1/branches/{id}PUT /api/v1/locations/{id}PUT /api/v1/pricing/category-rules/{id}PUT /api/v1/users/{sub}/home-branchEvery STATED LIMIT entry:
DELETE /api/v1/admin/settings/aiDELETE /api/v1/admin/settings/routingDELETE /api/v1/delivery/drivers/{id}DELETE /api/v1/delivery/vehicles/{id}DELETE /api/v1/edi/partners/{id}GET /api/v1/admin/modulesGET /api/v1/admin/settings/aiGET /api/v1/admin/settings/routingGET /api/v1/appsGET /api/v1/branchesGET /api/v1/branches/{id}GET /api/v1/configurator/optionsGET /api/v1/configurator/presetsGET /api/v1/configurator/rulesGET /api/v1/delivery/driversGET /api/v1/delivery/drivers/{id}GET /api/v1/delivery/vehiclesGET /api/v1/delivery/vehicles/{id}GET /api/v1/edi/partnersGET /api/v1/edi/partners/{id}GET /api/v1/edi/partners/{id}/catalogGET /api/v1/governance/rfcsGET /api/v1/governance/rfcs/{id}GET /api/v1/market-indicesGET /api/v1/market-indices/{id}/historyGET /api/v1/matching/configGET /api/v1/millwork/optionsGET /api/v1/millwork/options/{id}GET /api/v1/price_levelsGET /api/v1/pricing/categoriesGET /api/v1/pricing/rebates/programsGET /api/v1/pricing/rebates/programs/{id}GET /api/v1/pricing/rebates/programs/{id}/claimsGET /api/v1/unitsGET /api/v1/units/{code}GET /api/v1/vendorsGET /api/v1/vendors/{id}POST /api/v1/apps/{key}/disablePOST /api/v1/apps/{key}/enablePOST /api/v1/configurator/build-skuPOST /api/v1/configurator/validatePOST /api/v1/delivery/driversPOST /api/v1/delivery/drivers/{id}/photoPOST /api/v1/delivery/vehiclesPOST /api/v1/delivery/vehicles/{id}/photoPOST /api/v1/edi/partnersPOST /api/v1/edi/partners/{id}/import-catalogPOST /api/v1/governance/rfcsPOST /api/v1/governance/rfcs/{id}/transitionsPOST /api/v1/millwork/optionsPOST /api/v1/parsing/uploadPOST /api/v1/pricing/calculate-escalationPOST /api/v1/pricing/categoriesPOST /api/v1/pricing/rebates/programsPOST /api/v1/pricing/rebates/programs/{id}/claims/calculatePOST /api/v1/unitsPOST /api/v1/vendorsPOST /api/v1/vision/scanPUT /api/v1/admin/modules/{id}PUT /api/v1/admin/settings/aiPUT /api/v1/admin/settings/routingPUT /api/v1/delivery/drivers/{id}PUT /api/v1/delivery/vehicles/{id}PUT /api/v1/edi/partners/{id}PUT /api/v1/governance/rfcs/{id}PUT /api/v1/market-indices/{id}PUT /api/v1/matching/configPUT /api/v1/pricing/categories/{id}PUT /api/v1/units/{code}The wall dispatchers now match
r.Pattern(the registered pattern), not path suffixes;job_idresolves to its projects row's customer besidecustomer_id(both checked); the customer scoped reads that name a customer in a path, query or body are confined like the writes; and a write that would be dealer wide (a rule naming neither a customer nor a job, a tier category rule, a bulk delete naming a tier row) is refused. The GL, AP and bankrecon surfaces, the staff roster, the index refresh preview and the unfiltered pricing lists are REFUSED, reads and writes alike. ADR 0007 section 5.5 names the stated limits and advises which scopes not to mint on a bound key.Fix round 3 (rounds 5 and 6, the principal's rulings)
Every changed entry, its old class in brackets:
POST /pos/till/{id}/close,GET /pos/till/{id}/report,GET /pos/till/{id}/zreport,GET /pos/zreports,GET /pos/till/currentnow holdtill_sessions.branch_id/till_z_reports.branch_idin the repository (WALLED with its column named);GET /pos/returns,GET /pos/returns/{id}holdpos_returns.branch_id;POST /pos/transactions/{id}/itemsandDELETE .../items/{itemId}hold the transaction's branch with the insert never happening on a foreign transaction.GET /accounts/{id},GET /accounts/{id}/transactions,GET /ar/customers/{id}/statement(the customer branch wall on the path customer); REFUSED [was WALLED]GET /ar/reconciliation(its ledger half carries no branch fact).GET /pricing/calculatenow also resolvesjob_idtoprojects.customer_id(the pin's customer with the foreign customer's job is refused);POST /tax/previewis confined through a namedship_to_id's customer.PUT/DELETE /admin/settings/ai,PUT/DELETE /admin/settings/routing,PUT /admin/modules/{id},POST /apps/{key}/enable,POST /apps/{key}/disable,PUT /matching/config,POST/PUT/DELETEandPOST .../import-catalogon/edi/partners,POST /vendors,POST/PUT /units,POST/PUT /pricing/categories,POST /pricing/rebates/programs,POST /millwork/options,POST/PUTandPOST .../transitionson/governance/rfcs,PUT /market-indices/{id}, the vehicle writes; the reads stay stated limits.POST /products,PATCH /products/{id}/margins,/dimensions,/lead-time,PUT /products/{id}/kit-components,PUT /products/{id}/units,PUT /products/{id}/pim/content, the two PIM deletes,PATCH .../pim/media/{mediaId}/primary, the fourPOST .../pim/generate/*,POST/PUT /charge-codes,POST/PUT /payment-terms, andPOST /purchase-orders/refresh-reorder-targets; their reads are STATED LIMIT [was WALLED] (GET /productsand its detail, kit components, PIM reads and unit sets,GET /charge-codes,GET /payment-terms, the product links, the POS product search and catalog).POST /pricing/rules, the category rule writes (single, bulk, update, delete, bulk delete) and the tax exemption writes on it are refused unless the pin holds the customer alone, the refusal naming the other branch; the reads keep the shared rule.user.branch.granted,user.branch.revokedanduser.home_branch.setaudit rows with the key as the actor. PD-6: the parsing upload and the vision scan stay stated limits and each call records itsai.parsing.upload/ai.vision.scanaudit row with the key that spent the credit.DealerWideReads, andPUT /locations/{id}writingpathis probed and pinned: the path is a materialized breadcrumb with no enforcement, the branch comes from the parent at insert, and no read resolves a branch from a path, so a path naming the foreign branch cannot move a row under it.ADR 0007 section 5.5, the CONTRACT-CHANGES row
SEC-BOUND-KEY-PIN-3and the delivery, charge codes, products and locations module pages say the same. The pos handler file is untouched (PR 72 rewrites it): the pos confinement lives in the repository queries and the branch wall layer; the pos files changed areinternal/pos/till_repository.go,internal/pos/returns_repository.go,internal/pos/repository.goandinternal/pos/service.go.Fix round 4 (head 7116cc5)
Class counts are unchanged over the 345 machine key reachable census routes: WALLED 145, CONFINED 31, STATED LIMIT 51, REFUSED 118. No route was added or removed. No pos file changed in this round (till_repository.go, returns_repository.go, repository.go and service.go under internal/pos are untouched). refactor/v1 (3c178d7) merged first with no conflicts.
Changed entries and walls added:
customer_id(wall.payments); intent names no customer and passeswall.invoiceswall.orders; and for every caller the ship to and contact must belong to the order's customer (400 naming the field)wall.quoteswall.draftGuard)Shared customer fields (deny by default)
A field not listed as BRANCH LOCAL is RESTRICTED. A bound key on a customer held by its branch and another is refused on every RESTRICTED field (403, the PD-4 words naming the lowest numbered other branch, a key.branch_refused row, nothing changed); a customer held by the pin alone, a user and an unshared customer are unchanged; an unbound key changes them with a
customer.shared_restricted_changedrow naming the key and the before and after values.No other route writes a customer's record (the repository writes are create, update, salesperson and escalation policy; the AR balance posting runs through the walled payment and invoice routes).
Tests
Each fix was red first on 372b37a (the legs answered 2xx or 204 and wrote rows) and is green now; each is also proven by a mutant (about 30 applied one at a time, all red). The twin sweep sends valid bodies with an own twin control, measures by the twins' ids, and a coverage test fails by name for any WALLED route without a bound key leg. Migration 107 has a test. Open for the lead: a machine key carries no role, so the ruling's "finance or owner" condition for the unbound key cannot be evaluated; the unbound key is allowed and audited.
Fix round 5 (reviews 10 and 11, principal ruling item 48)
Class counts are unchanged over the 345 machine key reachable census routes: WALLED 145, CONFINED 31, STATED LIMIT 51, REFUSED 118. No route was added or removed. refactor/v1 (d9ecb2b) was already the merge base: the round opened already up to date, no conflicts. PR 72 has not merged: its resolve route is not in the allowlist and its branch is not merged.
Every route and wall added or changed this round:
wall.pos, the resolver posSyncCustomersOf): a replayed sale on a customer the pin does not hold is 403 naming customer_id, nothing written (its ACCOUNT tender would invoice that customer); the change is in the wall layer and the allowlist note only, no pos code touchedbodyDocumentCustomerover documentCustomersOf, the resolution the pricing calculate read carries)quoteRowCustomerOf), as the draft promotions recheck their payload: a quote a caller moved onto a foreign customer is refused where it would become an orderThe customers:restricted scope (item 48)
A shared customer's RESTRICTED fields change only with the finance or owner role (a user) or an unbound machine key carrying the dedicated
customers:restrictedscope. The scope is in the grant grammar (ValidScopeGrammar) though no route requires it, so no module scope and no default grant carries it; a mint naming no scopes grants none. Only an owner may mint it: the admin role gets a 403 naming the scope, and a caller with no claims fails closed; the mint'skey.createdaudit row names every granted scope, that one included. A sales user, an admin user and every unbound key without the scope are refused in the PD-4 words naming the lowest numbered branch holding the customer; a bound key is refused even with the scope. Thecustomer.shared_restricted_changedaudit row stays exactly as it was, written by the scoped key's change with the actor and the before and after values; a user's change writes no row. The customer routes' user guard admits finance beside admin, owner and sales so the ruling's role can reach the fields. The escalation audit's before and after values now carry agreement_signed_at beside the reference.Shared customer fields (deny by default, round 5 state)
The field classes are unchanged from round 4 (the table below stands); what changed is who may change a RESTRICTED field of a shared customer (the scope rules above) and one new row:
Tests
Each code fix was red first on 4bad569 and is green now (the pos sync leg showed the sale synced and the foreign invoice written; the job legs showed the foreign job stored and priced by its override; the convert leg showed the order created with the foreign exemption; the default add leg showed the shared default cleared; the activity legs showed the foreign contact written; the scope legs showed the mint refused by the grammar and every caller change open; the legacy row's PUT showed the 500). Each test gap is proven by a mutant: the credit memo PUT pattern dropped from the invoices mount, salesperson_id classed BRANCH LOCAL, a DESC on either also held by lookup (the shared customer now carries a third branch), RecordRequest removed from wall.customers, policyFacts without the signature date, and a route driven only as an A control failing the coverage by name (the walled route coverage now counts only the drivers' explicit twin marks). Left for a later item: the pre existing draft order promote 500 (round 11 P3-3, identical on the base). The exposure gate and override fix and its coverage test are kept (item 48).
Fix round 6
Review 13 (second review) and the principal's item 54 rulings. No route added or removed (the same 345 census routes).
COALESCE(l.branch_id, r.branch_id)in the transaction create, the till open and the return lookup, so a bound key's pos sync on a register whose location names no branch is held to the pin (it wrote an open transaction at the default branch). The till session now carries the register's branch (branch_idin its body, four pos goldens re-recorded, listed in CONTRACT-CHANGES); the seed movesREG-01onto Kelowna so seeded amounts are unchanged.customer.shared_restricted_refusedaudit row (actor, fields, branch, method, path).admin,owner,sales.key.branch_refusedrow).customers:restricted.SEC-BOUND-KEY-PIN-6and the customers and CRM activities pages state every changed rule.Fix round 7
Review 15 (the Opus second review) and the principal's item 57 rulings. No route added or removed (the same 345 census routes). Head d40efd1.
branch_idontill_sessions,till_z_reportsandpos_returnsfrom each row's own register (COALESCE(the register's location's branch, pos_registers.branch_id)), so a drawer open at upgrade is reachable and closable again and the legacy Z reports and returns come back to the branch scoped reads. It touches only rows whose branch is NULL, counts what it set per table in a NOTICE, reports by count and first ids any row whose register names no branch and leaves it as it is, moves no money, deletes nothing; its down leaves the data (the branch is a fact read from the row's own register).pos_sync_logrow is written.GET /customers/{id}andGET /customers/{id}/escalation-policy(reads only: the caller that must sendIf-Matchhas to be able to read the revision). Every other customer, contact, ship-to and activity route keeps the roles it had.REG-01upsert stays, limited to the seed's own row (the conflict is on the id) and to the fields the seed owns (the location and the branch), keeping a register's name, per the item 57 condition: a register created outside the seed is left exactly as it was by a re-seed.SEC-BOUND-KEY-PIN-6id in CONTRACT-CHANGES is resolved: the pos row and this round's rows areSEC-BOUND-KEY-PIN-7. ADR 0007 section 5.5, the allowlist notes and the customers and CRM activities pages state every changed rule.{"session":null}, close 409), the migration's NOTICE counts, untouched rows and idempotent rerun in a scratch database, the per-table serve legs, the sale's 500, the till open and return 201s at the wrong branch, the sync's 200 with the foreign log row, finance's two 403 reads, the old refusal words, and the re-seed renaming the register.Fix round 8
Review 16 (Sonnet, CHANGES: one P2, one P3) and review 17 (Opus second, CLEAN: three P3s). No route added or removed (the same 345 census routes). Head ae8f690.
DO UPDATEmoved a REG-01 an operator re-pointed back onto Kelowna (r16 P2; the lead reads item 57's "another seed" as migration 027). A re-seed leaves a re-pointed register exactly where its operator put it, a register still on the placeholder moves as before, a second run is a no-op, and the name was never touched. The test forces the row's migration shape first, so it runs on any database.SEC-BOUND-KEY-PIN-POS, each round's rows under their own id, this round'sSEC-BOUND-KEY-PIN-8(round 7 had left the pos row and its own row both onSEC-BOUND-KEY-PIN-7).GET /pos/till/{id}/reportanswers 404 for a session the caller's branch context hides, the same missing-row answer the Z report and return reads give, not a 500 (r17 P3-1); the spec entry and thepos_till_closegolden follow.register_idnames no register is refused 400 (a typed error naming the register) before any item runs and before thepos_sync_logrow, which never names an unknown register; a lookup failure answers an error rather than skipping the hold (r17 P3-3).X-Branch-Idacts and reads across branches (r17's note; the same on refactor/v1).openapi.yamlandschema.d.tsstate every changed rule.Fix round 9
Review 18 (Sonnet, CLEAN: two P3 notes) and review 19 (Opus second, CHANGES: one P2, two P3s). No route added or removed (the same 345 census routes). Includes the lead's merge of refactor/v1 (PR 99's AP and GL changes; the AP and GL routes are all REFUSED to a bound key and no pos, seed or middleware file of theirs is touched by the merge). Head after the round is pushed with the round 9 commits on 22643f9.
default_branch_idthe seed reads before it changes anything (r19 P2-1; FROM-PRINCIPAL 57 and 59 read migration 027 as another seed, so the operator's row is never moved). The placeholder names no locations row, so while REG-01 sits on it the register's branch comes frompos_registers.branch_idalone, and round 8's location-only guard let a re-seed move back a register an operator had re-pointed by its branch alone. Proven red first with the seed's own statement, then green with the statement and with the realcmd/seedover two runs per leg: a branch only, location only or both re-point never moves; a REG-01 still in the migrations' shape moves exactly once; a rerun changes nothing; names are kept; a register another hand made is untouched; a database whose default an operator changed keeps REG-01 where it is, the safe side.register_idis refused the same typed 400 as one naming a register that does not exist, before any item runs and before thepos_sync_logrow, which never again carries an empty register_id (r19 P3-1, red first: the batch answered 200error_count-free and wrote its log row under'').POST /pos/sync, and that is the right class: a register that does not exist is a bad request for anyone, not only a bound key (round 18's P3s; stated in CONTRACT-CHANGESSEC-BOUND-KEY-PIN-9, the spec entry and ADR 0007's sync passage). The ids that already repeat in CONTRACT-CHANGES on refactor/v1 (PR 99'sC4-1brows among them) predate this PR and are left alone: a whole file pass on citable ids is the principal's hygiene work.key.branch_refusedrows, which rise by exactly one, and reads the row back for the refused call's method, path and pin branch (the other branch is named in the refusal words, asserted beside it). The old checks were placeholders that could not fail; the new ones bite, proven by disabling the wall's audit write and watching the test fail. The customers page states the row.