Skip to content

admin: rate-limit + audit emit + access-log prefix scrub (defense-in-depth layers 3-5) - #58

Merged
mastermanas805 merged 1 commit into
masterfrom
feat/admin-hardening-fresh
May 13, 2026
Merged

mastermanas805 merged 1 commit into
masterfrom
feat/admin-hardening-fresh

Conversation

@mastermanas805

Copy link
Copy Markdown
Member

Summary

Three new defense-in-depth middleware layers on top of the existing
ADMIN_PATH_PREFIX (gate 1) + ADMIN_EMAILS (gate 2):

  • AdminRateLimit (gate 3): 30 req/min/fingerprint cap via Redis sliding
    window. Excess returns 403 (NOT 429) with a body byte-for-byte
    identical
    to the RequireAdmin allowlist-miss 403, so probes can't tell
    which gate denied them. Runs BEFORE RequireAdmin so invalid-JWT probes
    also burn slots. Fail-open on Redis errors.
  • AdminAuditEmit (gate 4): writes an admin.access audit_log row on
    EVERY hit — success AND 403. Captures email, ip, path_suffix,
    http_status, user_agent_brief, denied_by. path_suffix strips
    the secret prefix — the prefix MUST NOT land in audit_log.
  • LogScrubber (gate 5): slog handler wrapper replacing every occurrence
    of ADMIN_PATH_PREFIX with the literal <ADMIN> in string attributes
    and message bodies. Wired in main() outside of logctx.NewHandler so
    the scrub runs LAST. Empty prefix → pass-through, zero overhead.

Chain order on the admin group:

AdminRateLimit -> AdminAuditEmit -> RequireAdmin -> handler

Audit sits BEFORE RequireAdmin because RequireAdmin returns 403 directly
on rejection (no c.Next) — any middleware after would never see the
rejection path. Audit's internal c.Next() drives the rest of the chain
and observes the final status either way.

New audit kind: models.AuditKindAdminAccess = "admin.access".

Test plan

Tests in same PR. make test-unit green across all packages, including
TestAgentActionContract.

  • log_scrubber_test.go — 9 cases: URL attr scrub, message body scrub,
    empty-prefix passthrough, non-string attrs untouched, nested slog.Group,
    JWT-pattern regression check, free-function helper.
  • admin_rate_limit_test.go — 4 cases: 31st req returns 403,
    byte-for-byte body parity with allowlist-miss 403, fail-open on Redis
    down, fingerprint-independence (A's exhaustion doesn't affect B).
  • admin_audit_test.go — 5 cases: success writes row with full
    metadata, rate-limited 403 writes row with denied_by=rate_limit,
    allowlist-miss writes row with denied_by=allowlist_miss, path-suffix
    strip helper unit test, metadata no-prefix invariant assertion.

Iron-rule verification

The audit_log metadata MUST NEVER contain the admin prefix string. Every
audit test asserts this via assert.NotContains(t, raw, prefix, ...)
against the persisted metadata blob. A dedicated AdminAuditEnsureMetadataNoPrefix
helper is exported for cross-package regression checks.

The 403-for-rate-limit / 403-for-allowlist-miss body parity is enforced
by TestAdminRateLimit_403MatchesAllowlistMiss_ByteForByte with a strict
bytes.Equal check on the two response bodies.

🤖 Generated with Claude Code

…depth layers 3-5)

Three new middleware layers on the admin route prefix:

1. AdminRateLimit — 30 req/min/fingerprint cap via Redis sorted-set
   sliding window. Excess returns 403 (NOT 429) with a body
   byte-for-byte identical to the RequireAdmin allowlist-miss 403, so
   an attacker who knows the secret prefix can't distinguish
   "throttled" from "not on the allowlist." Runs BEFORE RequireAdmin
   so invalid-JWT probes also burn slots. Fail-open on Redis errors.

2. AdminAuditEmit — after-response middleware writing one admin.access
   audit_log row per hit (success AND 403). Metadata carries email,
   ip, path_suffix, http_status, user_agent_brief, and a denied_by
   reason (rate_limit / allowlist_miss). path_suffix is the URL with
   the secret prefix stripped — the prefix MUST NOT land in audit_log.

3. LogScrubber — slog handler wrapper that replaces every occurrence
   of ADMIN_PATH_PREFIX with the literal "<ADMIN>" in string
   attributes and message bodies. Wired in main() on the outside of
   logctx.NewHandler so the scrub runs LAST, after every context
   field is stamped. Empty prefix → pass-through (zero overhead).

Chain order on the admin group:
    AdminRateLimit -> AdminAuditEmit -> RequireAdmin -> handler

Audit sits BEFORE RequireAdmin because RequireAdmin returns 403
directly (no c.Next) on rejection — any middleware after it would
never see the rejection path. Audit's internal c.Next() drives the
rest of the chain and observes the final status either way.

New audit kind: models.AuditKindAdminAccess = "admin.access".

Tests:
  - log_scrubber_test.go (9 cases) — URL attr, message body, empty
    prefix passthrough, non-string attrs untouched, nested groups,
    JWT-pattern untouched regression, helper function.
  - admin_rate_limit_test.go (4 cases) — 31st req gets 403,
    byte-for-byte body parity with allowlist-miss, fail-open on
    Redis down, fingerprint independence.
  - admin_audit_test.go (5 cases) — success row, rate-limited 403
    row with denied_by=rate_limit, allowlist-miss row with
    denied_by=allowlist_miss, path-suffix strip helper, metadata
    no-prefix invariant.

make test-unit green across all packages.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@mastermanas805
mastermanas805 merged commit b3ac1ea into master May 13, 2026
mastermanas805 added a commit that referenced this pull request May 14, 2026
…obby_plus copy (#107)

Wave FIX-H wraps up the BugBash B36 backup-correctness items so customer-
facing restore stops being one accidental retry away from a destroyed
database.

#57/#Q45 — restore replay guard
  Adds models.HasInflightRestore; a second POST /restore for the same
  resource while a prior row is pending/running returns 409
  restore_in_progress + AgentActionRestoreInflight. Fail-CLOSED on DB
  error (concurrent pg_restore --clean races itself).

#58/#A2 — restore to a new DB
  Accepts optional target_resource_id. Worker restores into the target
  (same team only); audit row carries both source + target ids. Skips
  the destructive-ack ceremony because the agent already opted into a
  fresh database.

#59 — backup integrity
  Migration 043 adds a nullable sha256 TEXT column to resource_backups.
  Worker stamps the digest at finalize; restore handler verifies before
  pg_restore. NULL on legacy rows is logged + accepted (fail-open on
  pre-043 data).

#64/#Q46 — cross-tenant 404
  GetBackupByIDForTeam joins resources to scope by team; cross-tenant
  backup_id guess now returns 404 backup_not_found instead of
  400 backup_resource_mismatch. Matches FIX-B tenant-isolation posture.

#65/#Q47 — refund quota on failure
  New internal endpoint POST /internal/teams/:id/backup-quota/refund
  (WORKER_INTERNAL_JWT_SECRET HS256, fail-closed when unset). The
  worker calls this when a MANUAL backup fails terminally so the
  team's daily manual-backups counter is credited back.

#66/#Q48 — hobby agent_action points to Hobby Plus
  AgentActionRestoreRequiresHobbyPlus added. The Hobby-tier restore 402
  now nudges Hobby Plus ($19/mo, restore enabled) instead of skipping
  the customer past the cheapest restore-enabled plan onto Pro ($49).

#67/#Q49 — destructive ack required for in-place restore
  In-place restore (no target_resource_id) now requires
  destructive_acknowledgment: true in the body. pg_restore --clean drops
  every table — refusing without an explicit ack prevents an agent
  testing a backup from wiping a live customer DB.

#Q50 — RPO/RTO on /capabilities
  Adds rpo_minutes + rto_minutes per tier in plans.yaml (anonymous/free
  = 0/0, hobby/hobby_plus = 1440/30, pro/team = 60/15) wired into the
  /api/v1/capabilities matrix via plans.Registry.RPOMinutes/RTOMinutes
  (added in instant.dev/common/plans#13).

Tests
  - +5 backup_test.go cases: ReplayBlocked, TargetNewDB,
    RequiresDestructiveAck, HobbyAgentActionPointsToHobbyPlus,
    CrossTenantBackupID_404
  - All prior backup tests updated to include destructive_acknowledgment
  - Contract test enforces the four new agent_action constants

DO NOT TOUCH list respected — no edits to email/, dpop.go, circuit/.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mastermanas805 added a commit that referenced this pull request May 21, 2026
…obby_plus copy

Wave FIX-H wraps up the BugBash B36 backup-correctness items so customer-
facing restore stops being one accidental retry away from a destroyed
database.

#57/#Q45 — restore replay guard
  Adds models.HasInflightRestore; a second POST /restore for the same
  resource while a prior row is pending/running returns 409
  restore_in_progress + AgentActionRestoreInflight. Fail-CLOSED on DB
  error (concurrent pg_restore --clean races itself).

#58/#A2 — restore to a new DB
  Accepts optional target_resource_id. Worker restores into the target
  (same team only); audit row carries both source + target ids. Skips
  the destructive-ack ceremony because the agent already opted into a
  fresh database.

#59 — backup integrity
  Migration 043 adds a nullable sha256 TEXT column to resource_backups.
  Worker stamps the digest at finalize; restore handler verifies before
  pg_restore. NULL on legacy rows is logged + accepted (fail-open on
  pre-043 data).

#64/#Q46 — cross-tenant 404
  GetBackupByIDForTeam joins resources to scope by team; cross-tenant
  backup_id guess now returns 404 backup_not_found instead of
  400 backup_resource_mismatch. Matches FIX-B tenant-isolation posture.

#65/#Q47 — refund quota on failure
  New internal endpoint POST /internal/teams/:id/backup-quota/refund
  (WORKER_INTERNAL_JWT_SECRET HS256, fail-closed when unset). The
  worker calls this when a MANUAL backup fails terminally so the
  team's daily manual-backups counter is credited back.

#66/#Q48 — hobby agent_action points to Hobby Plus
  AgentActionRestoreRequiresHobbyPlus added. The Hobby-tier restore 402
  now nudges Hobby Plus ($19/mo, restore enabled) instead of skipping
  the customer past the cheapest restore-enabled plan onto Pro ($49).

#67/#Q49 — destructive ack required for in-place restore
  In-place restore (no target_resource_id) now requires
  destructive_acknowledgment: true in the body. pg_restore --clean drops
  every table — refusing without an explicit ack prevents an agent
  testing a backup from wiping a live customer DB.

#Q50 — RPO/RTO on /capabilities
  Adds rpo_minutes + rto_minutes per tier in plans.yaml (anonymous/free
  = 0/0, hobby/hobby_plus = 1440/30, pro/team = 60/15) wired into the
  /api/v1/capabilities matrix via plans.Registry.RPOMinutes/RTOMinutes
  (added in instant.dev/common/plans#13).

Tests
  - +5 backup_test.go cases: ReplayBlocked, TargetNewDB,
    RequiresDestructiveAck, HobbyAgentActionPointsToHobbyPlus,
    CrossTenantBackupID_404
  - All prior backup tests updated to include destructive_acknowledgment
  - Contract test enforces the four new agent_action constants

DO NOT TOUCH list respected — no edits to email/, dpop.go, circuit/.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant