Webhook Auth, Token Races & Fail-Closed Access Control

Introduction

When your SDK opens physical doors, a bug is not a rendering glitch — it is either a lock that will not open for a legitimate guest, or a door that opens for someone who should never have been let in. Both failure modes destroy trust, and they are caused by very different classes of defects.

This article walks through a batch of production changes in a smart-lock platform and extracts the generalizable engineering patterns behind them: multi-tenant webhook verification, session-scoped credential delivery, single-flight token refresh, a disciplined error taxonomy, default-deny RBAC, fail-closed feature flags, and how to test code that is intentionally random.

---

1. Multi-tenant webhooks: one endpoint, many secrets

A payment webhook that grants membership — and therefore booking and door access — is an authentication boundary, not a data pipe. The classic mistake is verifying every incoming event against a single global signing secret while serving many organizations.

The moment each tenant owns its own payment account, each tenant also owns its own signing secret. Verification must resolve the secret per organization, and must fail closed when the organization cannot be resolved.

export class WebhookError extends Error {
  constructor(public code: string, public status: number) {
    super(code);
  }
}
export async function verifyWebhook(req: Request): Promise<StripeEvent> {
  const signature = req.headers.get('stripe-signature');
  if (!signature) throw new WebhookError('missing_signature', 400);
  // Raw body BEFORE any JSON parsing — re-serialization breaks HMAC.
  const raw = await req.text();
  const orgId = resolveOrgFromRoute(req);
  if (!orgId) throw new WebhookError('unresolved_tenant', 404);
  const secret = await loadWebhookSecret(orgId);
  // No global fallback. Unknown tenant => reject, never "trust by default".
  if (!secret) throw new WebhookError('unknown_endpoint', 404);
  return stripe.webhooks.constructEvent(raw, signature, secret);
}

Three rules worth internalizing:

1. Verify against the raw body. Any middleware that parses and re-serializes JSON silently invalidates the HMAC.
2. No global fallback secret. A fallback turns a per-tenant boundary into a shared one; a leaked secret from one tenant then forges events for all of them.
3. Unresolvable tenant is a rejection, not a default. Fail closed on every branch of the resolution logic.

A related fix in the same area: subscription period fields moved from the subscription object to the subscription item. Reading current_period_end from the wrong level silently produced wrong expiry dates — which, in an access system, means credentials that live too long. Whenever an upstream API reshapes a field, prefer a parse step that throws over optional chaining that quietly yields undefined:

const periodEnd = subscription.items.data[0]?.current_period_end;
if (typeof periodEnd !== 'number') {
  throw new WebhookError('missing_period_end', 422);
}

---

2. The this-binding trap in security-critical call paths

One of the subtlest bugs in this batch: a database RPC helper was passed around as a bare method reference, losing its receiver.

// BROKEN: `rpc` loses its `this` binding and blows up at call time
const { rpc } = supabase;
await rpc('grant_membership_access', { p_user_id: userId });

This fails at runtime, inside a webhook handler, after the event has already been acknowledged — exactly the place where an exception is most expensive. Two defenses:

// 1. Wrap instead of destructure — the arrow preserves the receiver.
type RpcCall = <T>(fn: string, args: Record<string, unknown>) => Promise<{ data: T | null; error: DbError | null }>;
const rpc: RpcCall = (fn, args) => supabase.rpc(fn, args);

{
  "rules": {
    "@typescript-eslint/unbound-method": ["error", { "ignoreStatic": true }]
  }
}

The lint rule catches the whole class of defect statically. In privileged code paths — token minting, permission grants, credential dispatch — prefer explicit wrapper functions with declared types over passing methods around by reference.

---

3. Credential delivery endpoints are access-control endpoints

A "resend the key to a corrected phone number" endpoint looks like a convenience feature. It is actually the most dangerous route in the product: it re-targets a working credential to an attacker-supplied destination. If it only requires a transaction ID, anyone who can guess or observe that ID can hijack the key.

The fix is to bind the operation to the session that created the transaction:

export async function handleResend(req: Request): Promise<Response> {
  const { sessionToken, transactionId, phone } = ResendSchema.parse(await req.json());
  const session = await loadSession(sessionToken);
  if (!session || session.transactionId !== transactionId) {
    return json({ code: 'FORBIDDEN' }, 403);
  }
  if (session.expiresAt <= Date.now()) {
    return json({ code: 'SESSION_EXPIRED' }, 403);
  }
  if (await rateLimiter.exceeded(`resend:${transactionId}`, { max: 3, windowMs: 600_000 })) {
    return json({ code: 'TOO_MANY_REQUESTS' }, 429);
  }
  await auditLog({
    action: 'key.resend',
    transactionId,
    actorSessionId: session.id,
    destinationHash: sha256(normalizePhone(phone)),
    ip: clientIp(req),
  });
  return dispatchKeyDelivery(transactionId, phone);
}

Checklist for any endpoint that delivers or re-delivers a credential:

• Bind to a short-lived session token, not to a long-lived resource identifier.
• Verify the session owns the resource (session.transactionId === transactionId), not merely that a session exists.
• Rate limit per resource, because retries are the attack vector.
• Audit-log the destination as a hash — you need forensics without storing extra PII.
• Never log the credential itself (PIN, key URL, OTP) in plaintext.

The same principle drove a check-in fix where the active configuration was resolved from the recovery token rather than the facility default. A token carries the exact context that was issued; a default carries whatever the facility looks like now. When a token exists, it is the source of truth — otherwise a facility setting change can retroactively alter what a previously issued credential means.

---

4. Token refresh races: single-flight and honest error classification

Lock hardware integrations run on OAuth-style tokens that rotate. Two bugs appear together:

1. Concurrent refresh. N requests notice the token is expiring, all refresh, and the rotated tokens invalidate each other.
2. Misclassified errors. A 401 produced by an in-flight rotation is reported to operators as "lock disconnected".

The second is the trust killer: an operator who sees a false "device offline" alert starts distrusting every alert. Fix the race with single-flight, then classify errors honestly.

let refreshInFlight: Promise<AccessToken> | null = null;
export async function getAccessToken(): Promise<AccessToken> {
  const cached = await tokenStore.read();
  if (cached && !isExpiringWithin(cached, 60_000)) return cached;
  refreshInFlight ??= refreshToken(cached)
    .then(async (next) => {
      await tokenStore.write(next); // persist rotation before returning
      return next;
    })
    .finally(() => {
      refreshInFlight = null;
    });
  return refreshInFlight;
}

type FailureKind = 'auth' | 'transient' | 'fatal';
export function classify(err: unknown): FailureKind {
  if (isStatus(err, 401) || isStatus(err, 403)) return 'auth';
  if (isNetworkError(err) || isStatus(err, 429) || isStatus(err, 502) || isStatus(err, 503)) {
    return 'transient';
  }
  return 'fatal';
}
export async function callDevice<T>(op: () => Promise<T>): Promise<T> {
  try {
    return await op();
  } catch (err) {
    if (classify(err) === 'auth') {
      await getAccessToken(); // single-flight refresh, then one retry
      return op();
    }
    throw err;
  }
}

Rotation persistence deserves its own note: if the refreshed token is kept only in memory, every process restart burns a rotation and eventually desynchronizes from the provider. Persist before returning, and run the proactive refresh job on a schedule with a cutoff comfortably shorter than the token lifetime (for example, daily with a 5-day expiry window), so a single missed run never causes an outage.

Transient network faults deserve the same honesty elsewhere: a calendar sync that fails with a TLS handshake error should retry with backoff, while a 400 from a malformed payload should not be retried at all.

export async function withRetry<T>(op: () => Promise<T>, attempts = 3): Promise<T> {
  let lastErr: unknown;
  for (let i = 0; i < attempts; i++) {
    try {
      return await op();
    } catch (err) {
      lastErr = err;
      if (classify(err) !== 'transient') throw err;
      await sleep(2 ** i * 250 + Math.random() * 100); // jittered backoff
    }
  }
  throw lastErr;
}

---

5. An error taxonomy that never leaks a 500

Several fixes in this batch share one root cause: a policy outcome surfacing as a crash. Hitting a monthly booking quota returned 500. Re-subscribing to a plan hit a unique-constraint violation and returned 500. Resuming an interrupted payment returned 404.

Each of these is a known, expected business state. Map them explicitly:

Situation	Status	Client behaviour
Malformed input	400	Fix and retry
Not authenticated / wrong session	403	Re-authenticate
Policy limit reached (quota)	403	Show localized explanation
Already exists / duplicate action	409	Resume existing resource
Unexpected defect	500	Alert on-call

export class AppError extends Error {
  constructor(readonly detail: { code: string; status: number; i18nKey: string; meta?: Record<string, unknown> }) {
    super(detail.code);
  }
}
export async function subscribe(userId: string, planId: string) {
  try {
    return await db.insertMembership({ userId, planId });
  } catch (err) {
    if (isUniqueViolation(err, 'membership_user_plan_uniq')) {
      throw new AppError({
        code: 'MEMBERSHIP_ALREADY_ACTIVE',
        status: 409,
        i18nKey: 'errors.membership.already_active',
      });
    }
    throw err; // genuinely unexpected — let it page someone
  }
}

Idempotent resume, rather than a hard failure, for any multi-step money or credential flow:

export async function initiatePayment(input: InitiateInput): Promise<InitiateResult> {
  const pending = await findPendingIntent(input.membershipId);
  if (pending && !isExpired(pending)) {
    return { status: 'resumed', checkoutUrl: pending.checkoutUrl };
  }
  const created = await createIntent(input);
  return { status: 'created', checkoutUrl: created.checkoutUrl };
}

Two secondary benefits: 500 becomes a genuine alerting signal again, and every non-500 outcome carries an i18nKey so the message can be localized at render time and follow the user's language switch — instead of being frozen in whatever locale the server used when the error was created.

---

6. Default-deny RBAC and idempotent permission migrations

When a new capability ships (locker management, buyback management), the permission must exist before anyone can be granted it, and existing roles must be updated deliberately. A default-deny model means a missing seed manifests as "the menu is invisible" — annoying, but safe. The opposite default means a new capability is silently world-readable.

Permission migrations must be idempotent, because they will be replayed across environments and back-merged between branches:

insert into role_permissions (role_id, permission_key)
select r.id, p.key
from roles r
cross join (values ('locker-management'), ('buyback-management')) as p(key)
where r.scope = 'organization'
  and r.name in ('org_admin', 'org_manager')
on conflict (role_id, permission_key) do nothing;

Enforcement belongs in the data layer too, so a forgotten UI guard is not the only thing standing between a user and a locker:

create policy locker_boxes_read on locker_boxes
for select using (
  exists (
    select 1 from user_permissions up
    where up.user_id = auth.uid()
      and up.org_id = locker_boxes.org_id
      and up.permission_key = 'locker-management'
  )
);

---

7. Fail-closed feature flags as a rollout safety net

A large share of these commits are flag work: a new menu flag shipped default OFF, override-only, plus an admin view with grouping, filtering, and bulk disable across facilities. That last capability is the important one — a rollout mechanism without a fast, wide kill switch is not a safety net.

export interface FlagDefinition {
  key: FlagKey;
  defaultValue: boolean;
  overrideOnly?: boolean; // never on by default, even if defaultValue flips
}
export function isEnabled(key: FlagKey, ctx: FlagContext): boolean {
  const def = FLAG_DEFINITIONS[key];
  if (!def) return false; // unknown flag => disabled, never enabled
  const override = ctx.facilityOverrides?.[key];
  if (typeof override === 'boolean') return override;
  return def.overrideOnly === true ? false : def.defaultValue;
}

The invariants: unknown flag is false; evaluation never throws; overrides are scoped (facility/org) so a bad rollout is contained; and every flag change is audit-logged with actor and scope, because "who turned on the door-adjacent feature for that facility?" is a question you will be asked.

---

8. Guarding shell interpolation in CI

A CI job validated pull-request titles by interpolating the title directly into a shell command. A title containing a quote broke the script — and the same mechanism is a straightforward command-injection vector, because PR titles are attacker-controlled text.

- name: Validate PR title
  env:
    PR_TITLE: ${{ github.event.pull_request.title }}
  run: node scripts/validate-pr-title.mjs "$PR_TITLE"

The rule is simple and absolute: never interpolate untrusted template expressions into a run: block. Pass them through the environment, where the shell treats them as data rather than code. Your CI has credentials; treat it as production.

---

9. Testing code that is deliberately random

Locker PIN issuance picks a free box and generates a code. When the picking strategy changed from "first free" to a shuffle, a test asserting an exact box ID broke — a classic sign the test was asserting an implementation detail rather than a property.

Inject the randomness source and assert invariants:

export interface Rng {
  next(): number;
}
export async function issueLockerBoxPin(deps: { boxes: Box[]; rng: Rng }): Promise<Issued> {
  const free = deps.boxes.filter((b) => b.status === 'available');
  if (free.length === 0) {
    throw new AppError({ code: 'NO_AVAILABLE_BOXES', status: 409, i18nKey: 'errors.locker.no_capacity' });
  }
  const picked = free[Math.floor(deps.rng.next() * free.length)];
  return { boxId: picked.id, pin: generatePin(deps.rng) };
}

it('always picks from the free set, never an occupied box', async () => {
  const boxes = makeBoxes({ free: 3, occupied: 5 });
  for (let seed = 0; seed < 200; seed++) {
    const result = await issueLockerBoxPin({ boxes, rng: seededRng(seed) });
    expect(freeIds(boxes)).toContain(result.boxId);
    expect(result.pin).toMatch(/^\d{6}$/);
  }
});
it('reports capacity exhaustion as 409, not a crash', async () => {
  const boxes = makeBoxes({ free: 0, occupied: 8 });
  await expect(issueLockerBoxPin({ boxes, rng: seededRng(1) })).rejects.toMatchObject({
    detail: { code: 'NO_AVAILABLE_BOXES', status: 409 },
  });
});

That second test mirrors a real fix: when every box is physically occupied, users deserve a capacity message, not a generic failure. "All resources busy" is a first-class domain state and should be modeled and tested as one.

One more testing note from this batch: external clients were mocked at the RPC boundary and environment validation moved to module load. Validating configuration once, at startup, converts a class of runtime surprises into a loud boot failure:

const EnvSchema = z.object({
  DEVICE_API_BASE_URL: z.string().url(),
  DEVICE_API_CLIENT_ID: z.string().min(1),
  DEVICE_API_CLIENT_SECRET: z.string().min(1),
});
export const env = EnvSchema.parse(process.env); // throws at import time, not mid-unlock

---

10. State transitions that cannot be silently rewritten

A guided provisioning flow revealed two familiar hazards: a double-submit that started a second run while the first was in flight, and a restart that blanked previously written records.

Model the flow as an explicit discriminated union, guard re-entry, and make superseded records immutable:

type ProvisionState =
  | { status: 'idle' }
  | { status: 'applying'; attempt: number }
  | { status: 'verifying'; appliedAt: number }
  | { status: 'applied'; snapshot: Snapshot; frozen: true }
  | { status: 'failed'; error: AppError };
export function reduce(state: ProvisionState, event: ProvisionEvent): ProvisionState {
  switch (state.status) {
    case 'idle':
      return event.type === 'START' ? { status: 'applying', attempt: 1 } : state;
    case 'applying':
      if (event.type === 'START') return state; // re-entrancy guard
      if (event.type === 'APPLIED') return { status: 'verifying', appliedAt: Date.now() };
      return event.type === 'FAILED' ? { status: 'failed', error: event.error } : state;
    case 'verifying':
      return event.type === 'VERIFIED'
        ? { status: 'applied', snapshot: event.snapshot, frozen: true }
        : state;
    case 'applied':
      return state; // terminal: a new run appends a record, never mutates this one
    case 'failed':
      return event.type === 'START' ? { status: 'applying', attempt: 1 } : state;
  }
}

The compiler enforces exhaustiveness, illegal transitions are inert rather than corrupting, and history becomes append-only — which is exactly what an audit trail requires. The same discipline applies to lifecycle side effects: when a plan is deactivated, the transition must atomically revoke member access and stop billing, so entitlement state and payment state can never disagree.

---

Summary

The common thread across all of these changes is that every ambiguous branch must resolve toward denial or toward an explicit, named state — never toward a silent default:

• Per-tenant webhook secrets, no global fallback, verification against the raw body.
• Credential re-delivery gated on a session that provably owns the resource, rate-limited and audit-logged.
• Single-flight, persisted token refresh with honest auth / transient / fatal classification.
• An error taxonomy where quotas are 403, duplicates are 409, and 500 is reserved for genuine defects.
• Default-deny RBAC with idempotent permission migrations and database-level enforcement.
• Fail-closed feature flags with a bulk kill switch and audit logs.
• Untrusted CI input passed via the environment, never interpolated into a shell.
• Property-based tests over injected randomness, and explicit state machines that make illegal transitions unrepresentable.

None of these is individually clever. Together they are the difference between a system that opens doors and a system you are willing to let open doors.