Reference
RBAC & security
The findings register: what was once possible, and what closed it.
docs/SECURITY_FINDINGS.md
Every security finding raised against this codebase, what it actually allowed, and what closed it.
Findings are never deleted from this table. A fixed blocker stays here with its original description, because the value of the record is knowing what was once possible — that is what tells the next person which assumptions to re-check when they touch the same code. Deleting a closed finding loses exactly the information that made it worth writing down.
Severity: Blocker (exploitable, ship-stopping) · High (exploitable with a precondition) · Medium (weakens a control) · Low (hygiene, defence-in-depth).
Status: 🔴 open · 🟡 accepted (known, tracked, not yet closed) · ✅ closed.
Closed
| # | Sev | Finding | What it allowed | Closed by | Date |
|---|---|---|---|---|---|
| S-001 | Blocker | OAuth state was signed but not bound to a browser. /start is unauthenticated, so an attacker could mint a genuinely-valid signed state at will. | Session fixation: attacker completes consent with their own account, captures code+state, gets a victim to open the callback — the victim's browser stores the attacker's session and starts connecting real ad accounts and channels to the attacker's organization. Via /link-url, worse: a state carrying linkUserId = attacker meant a victim's consent attached their GitHub identity and its repo-scoped token to the attacker's account, and the victim's later "Sign in with GitHub" landed them inside it. | HttpOnly, SameSite=Lax, __Host- nonce cookie set at /start and /link-url; state carries only hashToken(nonce); callback requires both and compares in constant time; cookie cleared before any work so a captured URL cannot be replayed. D-018, superseding D-012 — whose stated rationale was wrong. | 2026-07-29 |
| S-002 | Blocker | projectScope failed open on any unrecognised role. It was a deny-list (if agency or client, restrict) over an unconstrained String column. | Any role the check did not anticipate — a typo, a role added later, a hand-inserted row, "Agency" with a capital A — granted org-wide read of every project, its budgets and its whole team. This is the single control D-009 rests on. | Inverted to an allow-list of internal roles, so an unknown role is restricted by default; User.role made a Prisma enum so the column cannot hold an unanticipated value. Test asserts an unknown role sees zero projects. D-019. | 2026-07-29 |
| S-003 | High | Subscription plan read from metadata.plan. Metadata is written once at checkout and never updated by a plan change in the Stripe billing portal. | An organization that downgraded scale → starter kept scale limits indefinitely. Separately, ?? "starter" silently granted a paid plan to any subscription arriving without metadata. | Plan derived from the billing price via config.stripe.prices; an unknown price degrades to free and logs loudly. D-020. | 2026-07-29 |
| S-004 | High | Webhook idempotency was check-then-insert. | Two concurrent deliveries of one Stripe event both found no row and both applied. The loser's insert then violated the unique constraint, 500'd, and made Stripe retry an event that had already succeeded — a retry storm on a handler that worked. | The INSERT is the claim and happens before the work, so the primary key is the lock; P2002 is treated as "already processed". Tested with three concurrent deliveries. D-021. | 2026-07-29 |
| S-005 | High | Per-IP rate limiting was bypassable. clientIp trusted the first X-Forwarded-For hop with no trusted-proxy configuration. | Any caller could send X-Forwarded-For: <random> per request and never trip the limiter — making the login, signup and OAuth limits decorative, and brute-forcing free. | Uses the Nth hop from the right per TRUSTED_PROXY_COUNT; with no proxies configured the header is ignored entirely in favour of the socket address. | 2026-07-29 |
| S-006 | Medium | Unlimited endpoints: POST /auth/refresh (unauthenticated, does a JWT verify + DB read), /billing/checkout and /billing/portal (each a live Stripe call that can create a customer), /auth/oauth/:provider/link-url. | Free amplification against our database and our Stripe account. | limit(...) added to all four. | 2026-07-29 |
| S-007 | Medium | A signature failure and a missing API key were reported identically. stripe() throwing "not configured" sat inside the constructEvent try/catch. | A deployment receiving webhooks without an API key reported "Invalid signature" — sending whoever debugs it hunting a secret mismatch that does not exist. Found while writing the tests for S-004. | Webhook verification uses a client built only for that purpose; the client is constructed outside the try, so only constructEvent failing maps to 400. | 2026-07-29 |
| S-008 | Medium | Provider error bodies were written to logs. | OAuth token-exchange failures routinely echo the request back — including the client secret. A log is the hardest place to get a leaked credential back out of. | Log the host, path and status only. | 2026-07-29 |
| S-009 | Medium | Under-validated input reached Prisma and Intl. 2026-13-45 passed the date shape regex; 1$# passed currency: length(3); channels had no bound or dedupe; website_url accepted javascript:. | The date became an Invalid Date and surfaced as {"detail":"Internal server error"} instead of a 422. The currency threw RangeError inside Intl.NumberFormat and crashed the projects page for every user in the organization until the row was edited in the database. | Dates round-trip through Date; currency is /^[A-Za-z]{3}$/; channels bounded and de-duplicated; URLs restricted to http/https. formatMoney also falls back rather than throwing, so a legacy bad row cannot take a page down. | 2026-07-29 |
| S-010 | Medium | Unauthenticated health probe leaked AMQP connections. connect() checked channel before a long await with no in-flight dedupe. | N concurrent probes opened N broker connections, all but the last orphaned with no reference left to close them — a public endpoint that exhausts the broker's connection limit. | connect() memoises the in-flight promise; the probe reads cached state and never dials. | 2026-07-29 |
| S-011 | Medium | An explicit link could silently become a login as someone else. The existing-link branch was evaluated before the linkUserId branch. | A signed-in user linking a provider account already bound to another user was issued that other user's tokens. | 409 when a link is requested and the provider account belongs to a different user. | 2026-07-29 |
| S-012 | Low | Audit trail had holes. ipAddress existed on the table and was never written; nothing recorded login success, login failure, or OAuth unlink. | A burst of failed logins against one account was invisible — the trail only recorded successes. For an app that hosts external contractors, "who, and from where" is what the trail is read for. | recordAudit takes ipAddress; login success, login failure and unlink all recorded. | 2026-07-29 |
| S-013 | Low | Provider error query value forwarded verbatim to /login?error=…. | Not XSS (React escapes it), but arbitrary attacker-chosen copy rendered on our own login page. | Known codes mapped to fixed strings; everything else collapses to one message. | 2026-07-29 |
| S-014 | Low | errorHandler spread err.details after detail, and cast an arbitrary number to a status literal. | Supplementary details could overwrite the error message that is the actual contract; a non-HTTP status could reach the response. | detail spread last; status clamped to 400–599 or 500. | 2026-07-29 |
| S-015 | Medium | No refresh-token revocation. Logout cleared localStorage only; there was no denylist. | A leaked or stolen refresh token stayed valid for its full 14 days with no way to withdraw it. The threat model explicitly includes external contractors, which is exactly who that matters for. | tokenVersion on User, embedded in the refresh token and compared on use; POST /auth/logout bumps it. Tokens predating the field are treated as version 0, so no existing session broke. Access tokens are deliberately not revoked — that would mean a DB read per request; residual access is bounded by the 30-minute TTL. | 2026-07-29 |
| S-016 | Medium | ProjectMember.role enforced nothing. Validated, stored and serialised, but no authorization path read it. | A viewer and a lead had identical power inside a project. A field that looks like policy but is not is worse than no field, because a reviewer assumes the control is active. | assertCanWriteProject reads the membership role on project update and budget writes; viewer is read-only. Writing to a project you are not a member of returns 404, matching the read path so it cannot prove a project exists. | 2026-07-29 |
| S-017 | Low | Plan caps raced. assertWithinLimit read the count then acted on it. | Two simultaneous creates both read "0 of 1 used" and both succeeded, so a free organization could exceed its cap. | The count moved inside the create transaction at Serializable isolation. A serialization conflict (P2034) is retried up to three times, so the loser re-reads the committed count and gets a 402 explaining the cap rather than a 500 about a transaction. | 2026-07-29 |
| S-018 | Low | Retry backoff head-of-line blocked. One retry queue per kind with per-message TTLs. | RabbitMQ only dead-letters from the head, so a message parked for 80s held up every 5s retry queued behind it — jobs ran, but far later than intended. | One retry queue per delay tier, with the TTL on the queue rather than the message, so every message in a queue expires in arrival order. The tier is selected by routing key. | 2026-07-29 |
| S-019 | Low | Dead scaffolding read as implemented policy. requireRoles, assertPlatformAdmin, requirePlatformAdmin, assertPaidChannelsAllowed, decryptNullable — exported, never called. | No runtime risk, but the real cost: a reviewer sees an exported authorization helper and assumes the control is enforced somewhere. | The unused authorization helpers are deleted, each with a comment saying where the check actually lives. core/storage.ts and the unused serialisers are kept — a queued feature needs them — but now say plainly that nothing calls them yet. | 2026-07-29 |
| S-020 | High | Plan limits shown to customers but never enforced: seats and paid channels. No invite or accept path checked limits.seats, and nothing in ads/ read limits.paidChannels. | A free org could add unlimited users and connect ad accounts that spend money — the two limits the plan cards used to justify Starter. | assertSeatAvailable at invite (pending invitations reserve a seat) and at accept (under FOR UPDATE on the organization row); assertPaidChannelsAllowed at OAuth start, OAuth attach and POST /ad-accounts. src/test/billing-entitlements.test.ts. | 2026-09-18 |
| S-021 | High | Stripe unpaid mapped to our entitled past_due. | Once Stripe stopped retrying and marked a subscription unpaid, the org kept its paid plan indefinitely without paying. | unpaid is its own status, excluded by isEntitled. src/test/billing-webhook-lifecycle.test.ts. | 2026-09-18 |
| S-022 | Medium | The Instagram demo logged live access tokens. httpx logs every request at INFO with the full URL, and a Graph call carries access_token in the query string. | Each lookup wrote a working credential into the service's stdout — and so into the background-task log file and anything pasted from it. The token in question could read the connected Instagram account until it expired. | The httpx logger is raised to WARNING in services/instagram-competitor-demo/app/main.py; the service's own graph error line logs Meta's code and message, never the URL. Nothing in a response ever carried the token. | 2026-09-29 |
| S-023 | High | API key project and scopes are stored but never enforced. ApiKey.projectId and ApiKey.scopes are written at creation (apps/api/src/modules/api-keys/domain.ts), but apps/api/src/http/middleware.ts only sets the key's user and nothing reads the key's project or scopes. | A key created for one project with narrow scopes acts with its owner's full organisation access: every project, every write the owner can make. A leaked key is a leaked login. | Open. Found in the 2026-10-01 feature audit (F-072 moved to in progress). | 2026-10-01 |
Open and accepted
Known, tracked, and not yet closed. Each row names what would close it — a finding without that is a worry, not a task.
| # | Sev | Finding | Why it is survivable for now | What closes it |
|---|---|---|---|---|
| — | — | Nothing open. All 19 findings from the 2026-07-29 review are closed. |
The next review will add rows here. An empty table is a snapshot, not a claim that the code is finished being wrong.
How this register is used
- A new finding gets a row before it gets a fix. The row is the tracking; the commit closes it.
- Blockers stop the branch. Nothing merges to
devwith an open 🔴. - A closed row keeps its original description. Rewriting it to describe the fixed state destroys the reason the row exists.
- Every closed row names the commit-level change and, where one exists, the decision record.
- Findings map to features in FEATURES.md and to proofs in VERIFICATION.md; a fix without a test is not closed, only claimed.