Finding 3: the missing audit-log write
The first two findings showed themselves: a try/catch you could read, and an XSS sink that rendered <b>bold</b> as live HTML the moment you opened the invoice.
This one shows nothing. The app runs, the transfer succeeds, no error is thrown, no test breaks.
Your job is to document the silent ownership transfer: the mutation in src/lib/billing/transfer-ownership.ts that re-points the org’s owner without writing a single audit-log row, written up as findings/003-audit-log-ownership-transfer.md.
That invisibility is itself the consequence you have to put into words. The finding’s four sections name the rule (the canonical audit-log event set and its transaction discipline), the grep evidence for the gap, the consequence as an auditor and a customer would feel it, and the in-transaction fix with its exact event slug and redacted payload. You document the defect; you do not patch it.
Directoryfindings/
- 003-audit-log-ownership-transfer.md the only file you touch this lesson
Directorysrc/
Directorylib/
Directorybilling/
- transfer-ownership.ts read-only — the defect you document, never patch
Your mission
Section titled “Your mission”This finding hides best, because there is no running app to read; you catch it by holding a fixed list in your head and walking it against the code. The list is the canonical six-category audit-log event set from the audit-log policy lesson: auth, membership, billing, data-export, deletion, and ownership/tenancy. Every security-relevant mutation has to co-transact a write from that set, so you pattern-match each one against the list: this changes who controls the org, does it write an audit row?
The mechanic is grep-then-read, reused here against transactions instead of access checks.
Grep db.transaction across src/lib to enumerate every transactional mutation, then check each security-relevant one for a logAudit(tx, …) write riding the same transaction.
The seeded gap is the most extreme version: transferBillingOwnership imports no audit writer at all.
The grep also returns a legitimate site, the Stripe webhook in src/lib/webhooks/stripe.ts, which co-transacts its billing.subscription.* rows on every branch; a complete finding explains why that hit is not a defect.
Write the consequence in compliance and customer-facing terms: an auditor blind to the org’s ownership history, and an Activity page silent on the most consequential thing that can happen to a tenant. Never write it as “we forgot to log this” — that is a developer’s note, not a description of who gets hurt.
findings/003-audit-log-ownership-transfer.md has all four template sections — Rule, Location, Consequence, Fix — populated.src/lib/billing/transfer-ownership.ts.logAudit write riding tx, carrying the org.ownership-transferred event.transferBillingOwnership writes no audit row — proving you documented the defect rather than patching the read-only target.src/lib/webhooks/stripe.ts) the grep also returns and why it is not a finding.recentAuditLogs) is silent on it — with no “could potentially” hedging.org.ownership-transferred, single-dot entity.verb-pasttense), the write riding tx not the global db, and the redacted payload { previousOwnerId, nextOwnerId }.Coding time
Section titled “Coding time”Open findings/003-audit-log-ownership-transfer.md, copy the four sections from findings/template.md, and write the finding against the brief above before you read on. The grep is the same muscle as finding 1; the new muscle is the category cross-walk. With a draft in hand, expand the worked solution and compare.
Reference solution and walkthrough
Read the target the way an audit does: look for what is not there. The mutation is plain; the silence at the end of the transaction is the defect.
export const transferBillingOwnership = async ( orgId: string, previousOwnerId: string, nextOwnerId: string,): Promise<void> => { await db.transaction(async (tx) => { await tx .update(organization) .set({ ownerId: nextOwnerId }) .where(eq(organization.id, orgId));
// Demote the previous owner and promote the next one on their membership rows. await tx .update(member) .set({ role: 'admin' }) .where(eq(member.userId, previousOwnerId));
await tx .update(member) .set({ role: 'owner' }) .where(eq(member.userId, nextOwnerId));
// SEEDED #3: no in-transaction audit write here. The mutation lands silently. });};Three UPDATEs in one transaction: organization.ownerId moves, and the two membership rows swap roles. Control of billing and tenancy for the whole org changes hands atomically, and the transaction commits with no record of it. You cannot see the defect because the code does exactly what it should, minus the one thing nobody watches at runtime.
Here is the completed finding as it lands in the repo.
Category: Audit-log gaps (security baseline).
Severity: high — a tenancy-changing mutation lands with no audit row, so the most consequential event a tenant can experience leaves no trace. It is high rather than critical because the transfer itself is correct (access is not bypassed here — finding 1 owns that): the loss is the record, not the gate.
Every security-relevant mutation co-transacts an audit-log write. The canonical six-category event set (auth, membership, billing, data-export, deletion, ownership/tenancy) each mandates a row, and the write rides the same transaction as the mutation, so a committed change can never exist without its audit record (chapter 081, lesson 3 — the canonical event set and transaction discipline; the entity.verb-pasttense single-dot naming convention).
Location
Section titled “Location”src/lib/billing/transfer-ownership.ts:
transferBillingOwnership— thedb.transactionat lines 27–45 re-pointsorganization.ownerId(lines 28–31) and rewrites both membership owner rows (lines 33–42), then closes at lines 44–45 with nologAudit(tx, …)call.
How it surfaced — two greps, cross-walked against the canonical event set: find every transactional mutation, then check each against the categories that mandate a row.
# 1. Every db.transaction in the lib — the mutations that must each carry an audit write.rg -n "db.transaction" src/lib --glob '*.ts'# 2. The mutating verbs inside them — does an UPDATE to a tenancy column ride an audit row?rg -n "\.update\(" src/lib/billing/transfer-ownership.tsGrep 1 returns two transactional sites: src/lib/webhooks/stripe.ts (legitimate — it co-transacts billing.subscription.* rows on every branch) and src/lib/billing/transfer-ownership.ts. Grep 2 lands three .update( calls — organization.ownerId and the two member.role rewrites — all inside the transaction. rg "logAudit" src/lib/billing/transfer-ownership.ts returns nothing, while src/lib/invitations/manage.ts co-transacts member.role-changed on the lesser mutation of demoting a non-owner. The cross-walk is what scores this: changing the org’s owner is an ownership/tenancy event, that category mandates a row, and the mutation writes none — a finding, not a “looked unusual” note.
Consequence
Section titled “Consequence”The most security-relevant event a tenant can experience — control of billing and tenancy moving from one account to another — happens and leaves no record. For an auditor reconstructing who held ownership when (a SOC 2 access-review, a breach forensic timeline, a customer dispute over who authorized a billing change), the ownership-transfer history is unrecoverable: audit_logs, the system of record for exactly this, has a hole where the row should be. The customer-facing surface lies by omission too — the Activity page driven by recentAuditLogs shows invitations sent, roles changed, and subscriptions activated, but stays silent on the one change that handed someone else control of the org, so a legitimate owner who lost their org sees nothing in their feed to explain it.
Add the in-transaction audit write to the db.transaction block in transferBillingOwnership, modeled on the member.role-changed write already shipping in src/lib/invitations/manage.ts. The slug is org.ownership-transferred — single-dot entity.verb-pasttense, the canonical form, and the exact slug the admin-side src/lib/admin/transfer-ownership.ts already uses, so the two transfer paths land one event name rather than two drifting ones. The write rides tx, never the global db, so it commits or rolls back atomically with the ownership change — logAudit takes the transaction as its first argument precisely so an off-transaction write fails to typecheck. The payload is redacted to the two ids the event is about: no emails, no roles, no PII.
await db.transaction(async (tx) => { await tx.update(organization).set({ ownerId: nextOwnerId }).where(eq(organization.id, orgId)); // …the two member.role rewrites… await logAudit(tx, { action: 'org.ownership-transferred', subjectType: 'organization', subjectId: orgId, payload: { previousOwnerId, nextOwnerId }, });});The write takes tx, not the global db, so it commits or rolls back atomically with the ownership change. logAudit’s signature takes the Transaction as its first argument with no bare-db overload, precisely so an off-transaction write — a change that lands without its audit row — fails to typecheck.
await db.transaction(async (tx) => { await tx.update(organization).set({ ownerId: nextOwnerId }).where(eq(organization.id, orgId)); // …the two member.role rewrites… await logAudit(tx, { action: 'org.ownership-transferred', subjectType: 'organization', subjectId: orgId, payload: { previousOwnerId, nextOwnerId }, });});The canonical single-dot entity.verb-pasttense slug, and the same one the admin-side src/lib/admin/transfer-ownership.ts already emits, so both transfer paths write one event name rather than two that drift apart.
await db.transaction(async (tx) => { await tx.update(organization).set({ ownerId: nextOwnerId }).where(eq(organization.id, orgId)); // …the two member.role rewrites… await logAudit(tx, { action: 'org.ownership-transferred', subjectType: 'organization', subjectId: orgId, payload: { previousOwnerId, nextOwnerId }, });});The payload is redacted to the two ids the event is about. No emails, no roles, no PII — an audit row records what changed, not a copy of the data.
Add org.ownership-transferred to the canonical event set’s documented catalog so the category is explicit, not implied.
It helps to see the gap rather than describe it. The same mutation shape ships twice in this codebase — silent in the billing transfer, complete in the role-change action. Read them side by side.
await db.transaction(async (tx) => { await tx .update(organization) .set({ ownerId: nextOwnerId }) .where(eq(organization.id, orgId));
// Demote the previous owner and promote the next one on their membership rows. await tx .update(member) .set({ role: 'admin' }) .where(eq(member.userId, previousOwnerId));
await tx .update(member) .set({ role: 'owner' }) .where(eq(member.userId, nextOwnerId));
// SEEDED #3: no in-transaction audit write here. The mutation lands silently.});The transaction commits with no logAudit call. The mutation that changes who controls the whole org leaves no trace.
await withTenant(ctx.orgId, async (tx) => { await tx .update(member) .set({ role: newRole }) .where( and(eq(member.id, memberId), eq(member.organizationId, ctx.orgId)), ); await logAudit(tx, { action: 'member.role-changed', subjectType: 'member', subjectId: memberId, payload: { before: target.role, after: newRole }, });});The logAudit(tx, …) write rides the same transaction as the role change. This is the lesser mutation — demoting a non-owner — and it still co-transacts its row. The billing transfer is the greater mutation and writes nothing; that asymmetry is the finding.
Two decisions are worth stating outright.
The event set is a project-level invariant, not a per-feature call. If each feature decides for itself whether a mutation is “worth” logging, the audit log records what developers happened to remember, not what happened. A fixed six-category set removes the judgment: a mutation touches one of those categories, so it writes a row. That is what makes the audit grep-able — you check each mutation against a list, not against a developer’s taste.
Severity is high, not critical. The transfer itself is correct; access is not bypassed, and finding 1 owns the access-control defect. What is lost is the record, not the gate. A missing audit row on a correct mutation is serious — the event is consequential and the log is its only system of record — but it sits one notch below a defect that lets an unauthorized actor through. Calibrating severity to the actual blast radius, rather than stamping everything critical, is what keeps a findings report worth reading.
The named risk class for this exact defect — a security-relevant change that lands with no audit row. Grounds the Rule and the severity call.
A CPA/CISA audit firm on what an auditor reads — who did what, when. The compliance lens your Consequence section is written toward.
Moment of truth
Section titled “Moment of truth”Run the lesson gate:
pnpm test:lesson 4A passing run looks like this:
✓ tests/lessons/Lesson 4.test.ts (5) ✓ Lesson 4 — Finding 003: the missing audit-log write (5) ✓ Requirement 1 — all four template sections are populated ✓ Requirement 2 — the Rule names the audit-log canonical event set with transaction discipline ✓ Requirement 3 — the Location names a grep command and the defect file ✓ Requirement 4 — the Fix names the in-transaction logAudit write with the canonical slug ✓ Requirement 5 — source-shape probe: the seeded defect is still present
Test Files 1 passed (1) Tests 5 passed (5)Requirements 1 through 4 read the observable shape of your finding: four sections populated, the Rule naming the audit-log event set with transaction discipline and citing the lesson, the Location naming a grep command and the defect file, the Fix naming the in-transaction logAudit write with the org.ownership-transferred slug.
Requirement 5 is a source-shape probe in the other direction: it asserts transferBillingOwnership still writes no audit row.
So a passing gate proves you documented the defect rather than patched the read-only target; add the logAudit call yourself and requirement 5 fails.
The gate cannot read for meaning, so confirm the rest by hand.
src/lib/webhooks/stripe.ts site, and says in one line why that hit is not a finding (it co-transacts billing.subscription.* rows on every branch).org.ownership-transferred, says the write rides tx not the global db, and gives the redacted payload schema { previousOwnerId, nextOwnerId } with no emails, roles, or PII.With finding 3 written, three of the eight categories are covered: the fail-closed bypass, the XSS sink, and the silent ownership transfer. Finding 4 leaves the application code entirely and reads the headers the app ships.