Skip to content

Commit 04c9fbb

Browse files
committed
sec(api): strip /healthz filename + drop env-var names from public 401s (API-21/90/217)
BUG-API-090 + BUG-API-217 — /healthz is anon-reachable and emits the raw migration filename, e.g. "063_forwarder_sent_audit_link.sql". That literally tells an attacker which feature shipped at which migration number — recon a public probe should not provide. Add migrations.State.PublicVersion() that strips to the numeric prefix ("063"). Canaries keep the commit_id+count+version drift tuple, attackers learn nothing about the schema. migration_count and migration_status unchanged. BUG-API-021 (plus siblings on webhook_secret_mismatch and webhook_signature_mismatch) — the public 401 envelope returned by /webhooks/brevo/<wrong-secret> literally named the BREVO_WEBHOOK_SECRET env var in agent_action. Same recon-leak problem: anyone probing the public URL learned the exact env-var name. Drop env-var vocabulary from all three webhook-config error agent_actions; point at the docs page where the operator-side rotation procedure lives. Wire contract preserved: error codes, statuses, messages unchanged — only agent_action sentences are softened. No new endpoints, no new fields, no auth changes. Surface checklist (rule 22): - api/internal/migrations/state.go — PublicVersion helper + Strings import - api/internal/router/router.go — /healthz emits PublicVersion() - api/internal/router/healthz_test.go — pinned shape updated + new regression - api/internal/handlers/helpers.go — 3 agent_action sentences softened - api/internal/handlers/brevo_webhook_test.go — explicit assertion that BREVO_ env names never reach wire - OpenAPI / dashboard / marketing — no surface change (envelope shape unchanged) Coverage block: Symptom: /healthz.migration_version "063_forwarder_sent_audit_link.sql"; brevo 401 names BREVO_WEBHOOK_SECRET Enumeration: rg -F 'BREVO_WEBHOOK_SECRET' internal/handlers/helpers.go ; rg -F 'mstate.Filename' internal/router/ Sites found: 3 agent_action strings + 1 router emit site Sites touched: 4 / 4 Coverage test: TestHealthzMigrationVersionStripsFilenameSuffix (asserts no '_' or '.sql' in output across 7 cases); brevo_webhook_test BREVO_* + BREVO_WEBHOOK_SECRET assertion Live verified: pre-merge: curl https://api.instanode.dev/healthz returned "063_forwarder_sent_audit_link.sql"; curl https://api.instanode.dev/webhooks/brevo/x returned "BREVO_WEBHOOK_SECRET" in agent_action. post-merge: verify in PR comment. Local gate: - go build ./... PASS - go vet ./... PASS - go test ./internal/migrations/ PASS - go test ./internal/router/ PASS - go test -run 'Brevo' ./internal/handlers/ PASS Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 61faeb1 commit 04c9fbb

5 files changed

Lines changed: 117 additions & 10 deletions

File tree

internal/handlers/brevo_webhook_test.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,8 +201,8 @@ func TestBrevoTxWebhook_SecretMismatch_AgentActionMentionsBrevo(t *testing.T) {
201201
if !strings.Contains(body, `"error":"brevo_secret_mismatch"`) {
202202
t.Errorf("body must carry error=brevo_secret_mismatch; got %s", body)
203203
}
204-
// Agent action must mention Brevo / BREVO_WEBHOOK_SECRET — NOT
205-
// "INSTANODE_TOKEN" or the generic login-recovery script.
204+
// Agent action must mention Brevo — NOT "INSTANODE_TOKEN" or the
205+
// generic login-recovery script.
206206
if !strings.Contains(strings.ToLower(body), "brevo") {
207207
t.Errorf("agent_action must mention Brevo; got %s", body)
208208
}
@@ -212,6 +212,17 @@ func TestBrevoTxWebhook_SecretMismatch_AgentActionMentionsBrevo(t *testing.T) {
212212
if strings.Contains(body, "log in at https://instanode.dev/login to mint a new one") {
213213
t.Errorf("agent_action must NOT carry the user-login recovery script; got %s", body)
214214
}
215+
// BUG-API-021 (QA 2026-05-29): the 401 envelope is reachable by any
216+
// brute-forcer probing /webhooks/brevo/<guess>. The pre-fix copy
217+
// literally named the BREVO_WEBHOOK_SECRET env var — recon a public
218+
// 401 must not provide. Assert the env-var name (and BREVO_ prefix
219+
// generally) never reaches the wire.
220+
if strings.Contains(body, "BREVO_WEBHOOK_SECRET") {
221+
t.Errorf("BUG-API-021: agent_action must NOT name BREVO_WEBHOOK_SECRET env var on a public 401; got %s", body)
222+
}
223+
if strings.Contains(body, "BREVO_") {
224+
t.Errorf("BUG-API-021: agent_action must NOT carry any BREVO_-prefixed env-var name; got %s", body)
225+
}
215226
}
216227

217228
// ── 4. Closed-by-default: empty configured secret OR empty URL param

internal/handlers/helpers.go

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -152,8 +152,18 @@ var codeToAgentAction = map[string]errorCodeMeta{
152152
// dashboard webhook URL contains the configured BREVO_WEBHOOK_SECRET).
153153
// API-6 (QA 2026-05-29): give this error its own copy. Follows the U3
154154
// contract — "Tell the user" opening, https://instanode.dev/ URL, < 280 chars.
155+
// BUG-API-021 (QA 2026-05-29): the pre-fix agent_action literally
156+
// named the BREVO_WEBHOOK_SECRET env var, which (a) is informational
157+
// disclosure to a brute-forcer probing the public 401 surface — they
158+
// learn the exact env-var name the operator must rotate — and (b)
159+
// targets the operator using internal vocab the calling agent has no
160+
// business surfacing to the end-user. The new copy drops the env-var
161+
// name and points at the public docs page (which documents the
162+
// rotation procedure for an operator that follows the link). Wire
163+
// contract preserved: error code, status, message all unchanged —
164+
// only the agent_action sentence is softened.
155165
"brevo_secret_mismatch": {
156-
AgentAction: "Tell the user this is a Brevo-webhook config mismatch, not their auth. Operators must verify the Brevo dashboard webhook URL matches the configured BREVO_WEBHOOK_SECRET — see https://instanode.dev/docs/email.",
166+
AgentAction: "Tell the user this is a Brevo-webhook configuration mismatch, not their auth — they have no action. Operators: rotate the Brevo webhook secret and update the api Deployment — see https://instanode.dev/docs/email.",
157167
},
158168
// webhook_secret_mismatch is the generic per-provider webhook URL-path-token
159169
// or shared-secret mismatch surface. API-19/96/97/98 (QA 2026-05-29): the
@@ -164,8 +174,12 @@ var codeToAgentAction = map[string]errorCodeMeta{
164174
// distinguishes the secret-not-configured branch from the signature-mismatch
165175
// branch below. Operator must wire the corresponding env var
166176
// (BREVO_WEBHOOK_SECRET / SES_SNS_SUBSCRIPTION_ARN) before the route accepts.
177+
// BUG-API-021 sibling: "set the corresponding webhook secret env var"
178+
// pointed at an internal env-var by name. Same softening as
179+
// brevo_secret_mismatch above — drop env-var vocab from the public
180+
// 401 surface; the docs page covers operator-side wiring.
167181
"webhook_secret_mismatch": {
168-
AgentAction: "Tell the user this is an email-webhook secret-config mismatch, not their auth. Operators must set the corresponding webhook secret env var in the api Deployment — see https://instanode.dev/docs/email.",
182+
AgentAction: "Tell the user this is an email-webhook configuration mismatch, not their auth — they have no action. Operators: configure the webhook secret in the api Deployment — see https://instanode.dev/docs/email.",
169183
},
170184
// webhook_signature_mismatch is the per-provider signature-verification
171185
// failure surface — the secret IS configured, the inbound payload's HMAC /
@@ -174,8 +188,11 @@ var codeToAgentAction = map[string]errorCodeMeta{
174188
// the secret yet" from "someone is sending bad signatures (or the provider
175189
// rotated keys)" without an operator hand-grepping log lines. Used by
176190
// /api/v1/email/webhook/brevo + /api/v1/email/webhook/ses.
191+
// BUG-API-021 sibling: "the api Deployment's env var" leaked the
192+
// internal wiring; soften to "the configured webhook secret" so the
193+
// 401 stays self-explanatory without naming env-var keys.
177194
"webhook_signature_mismatch": {
178-
AgentAction: "Tell the user the inbound email-webhook signature did not verify. Operators must confirm the dashboard webhook secret matches the api Deployment's env var and that the provider hasn't rotated signing keys — see https://instanode.dev/docs/email.",
195+
AgentAction: "Tell the user the inbound email-webhook signature did not verify. Operators: confirm the dashboard webhook secret matches the configured value and the provider hasn't rotated signing keys — see https://instanode.dev/docs/email.",
179196
},
180197
// webhook_method_not_allowed surfaces the GET-on-a-POST-only webhook URL
181198
// path (BUG-API-098). Brevo's dashboard sometimes sends a GET pre-flight to

internal/migrations/state.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ package migrations
2222
import (
2323
"context"
2424
"database/sql"
25+
"strings"
2526
"sync"
2627
"time"
2728
)
@@ -43,6 +44,35 @@ type State struct {
4344
Count int // total rows in schema_migrations; 0 when unknown
4445
}
4546

47+
// PublicVersion returns the migration-version string safe to surface on
48+
// the public /healthz envelope (BUG-API-090/217). The raw Filename leaks
49+
// the table/feature names embedded in the migration filename ("e.g.
50+
// 063_forwarder_sent_audit_link.sql" tells an attacker that a
51+
// forwarder_sent → audit_log FK landed at migration 63), which is
52+
// recon a reachable-by-anyone health probe should not provide. We strip
53+
// the filename to its numeric prefix only ("063"), which preserves the
54+
// canary contract (canaries compare commit_id+migration_count+
55+
// migration_version to a known-good tuple) without leaking domain
56+
// knowledge.
57+
//
58+
// Empty filename (DB unreachable / pre-migration) returns "" — same
59+
// shape the pre-fix wire emitted in the unknown branch.
60+
func (s State) PublicVersion() string {
61+
if s.Filename == "" {
62+
return ""
63+
}
64+
// Split on first "_" — migration filenames are `<3-digit>_<name>.sql`.
65+
// We tolerate a missing "_" (treat the whole stem as the version) and
66+
// a non-numeric prefix (return as-is) so a future renaming scheme
67+
// degrades to "leak the prefix only" rather than panicking. The
68+
// .sql suffix is stripped if present.
69+
stem := strings.TrimSuffix(s.Filename, ".sql")
70+
if idx := strings.IndexByte(stem, '_'); idx >= 0 {
71+
return stem[:idx]
72+
}
73+
return stem
74+
}
75+
4676
// Reader caches one State per process with a TTL. Safe for concurrent use.
4777
// Clock is injectable so tests can advance time without sleeping.
4878
type Reader struct {

internal/router/healthz_test.go

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func TestHealthzShape(t *testing.T) {
5151
"commit_id": buildinfo.GitSHA,
5252
"build_time": buildinfo.BuildTime,
5353
"version": buildinfo.Version,
54-
"migration_version": m.Filename,
54+
"migration_version": m.PublicVersion(),
5555
"migration_count": m.Count,
5656
"migration_status": m.Status,
5757
})
@@ -82,8 +82,13 @@ func TestHealthzShape(t *testing.T) {
8282
require.Equal(t, buildinfo.Version, got["version"])
8383

8484
// Migration contract — new fields the canary reads to detect drift
85-
// between binary commit and DB schema state.
86-
require.Equal(t, "022_schema_migrations.sql", got["migration_version"])
85+
// between binary commit and DB schema state. BUG-API-090/217: the
86+
// public envelope emits the numeric prefix only ("022"), never the
87+
// full filename ("022_schema_migrations.sql") which would leak the
88+
// embedded table/feature names.
89+
require.Equal(t, "022", got["migration_version"])
90+
require.NotContains(t, got["migration_version"], "schema_migrations",
91+
"BUG-API-090: migration_version must not leak filename suffix to anon /healthz callers")
8792
require.Equal(t, float64(22), got["migration_count"]) // JSON numbers decode as float64
8893
require.Equal(t, "ok", got["migration_status"])
8994

@@ -110,7 +115,7 @@ func TestHealthzMigrationStatusUnknownWhenDBDown(t *testing.T) {
110115
m := reader.Get(c.UserContext())
111116
return c.JSON(fiber.Map{
112117
"ok": true,
113-
"migration_version": m.Filename,
118+
"migration_version": m.PublicVersion(),
114119
"migration_count": m.Count,
115120
"migration_status": m.Status,
116121
})
@@ -129,6 +134,42 @@ func TestHealthzMigrationStatusUnknownWhenDBDown(t *testing.T) {
129134
require.Equal(t, float64(0), got["migration_count"])
130135
}
131136

137+
// TestHealthzMigrationVersionStripsFilenameSuffix is the BUG-API-090/217
138+
// regression. The pre-fix /healthz handler emitted the raw filename
139+
// ("063_forwarder_sent_audit_link.sql") which leaks the table+feature
140+
// names embedded in the migration to any anonymous probe. The public
141+
// envelope now emits the numeric prefix only — canaries still get a
142+
// drift signal, attackers don't learn the schema.
143+
func TestHealthzMigrationVersionStripsFilenameSuffix(t *testing.T) {
144+
cases := []struct {
145+
filename string
146+
want string
147+
}{
148+
{"063_forwarder_sent_audit_link.sql", "063"},
149+
{"022_schema_migrations.sql", "022"},
150+
{"001_init.sql", "001"},
151+
{"100_team_deletion_purge.sql", "100"},
152+
// No underscore — return stem only (still strips .sql).
153+
{"baseline.sql", "baseline"},
154+
// No extension — return as-is up to first underscore.
155+
{"063_anything", "063"},
156+
// Empty (DB unreachable / pre-migration) — empty out.
157+
{"", ""},
158+
}
159+
for _, tc := range cases {
160+
s := migrations.State{Filename: tc.filename}
161+
got := s.PublicVersion()
162+
require.Equal(t, tc.want, got,
163+
"BUG-API-090: PublicVersion(%q) must strip to numeric prefix; got %q",
164+
tc.filename, got)
165+
// Sanity rail: no underscore or .sql escapes the helper.
166+
require.NotContains(t, got, "_",
167+
"BUG-API-090: PublicVersion must never contain '_' (would leak table/feature name)")
168+
require.NotContains(t, got, ".sql",
169+
"BUG-API-090: PublicVersion must never contain '.sql' (would leak filename)")
170+
}
171+
}
172+
132173
// TestHealthzMigrationVersionMatchesEmbeddedFile is the sanity rail —
133174
// whatever filename the DB reports must exist in the binary's embedded
134175
// migration set. If the running pod returns "099_phantom.sql" but no

internal/router/router.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,13 +349,21 @@ func NewWithHooks(cfg *config.Config, db *sql.DB, rdb *redis.Client, geoDbs *mid
349349
migrationReader := migrations.NewReader(db, 0, nil)
350350
app.Get("/healthz", func(c *fiber.Ctx) error {
351351
mstate := migrationReader.Get(c.UserContext())
352+
// BUG-API-090 / BUG-API-217 (QA 2026-05-29): /healthz is a
353+
// public probe (no auth, no rate-limit by token). Emitting the
354+
// raw migration filename leaked the table/feature names embedded
355+
// in it ("063_forwarder_sent_audit_link.sql" tells an attacker a
356+
// forwarder_sent→audit_log FK landed at migration 63). Strip to
357+
// the numeric prefix only via State.PublicVersion so canaries
358+
// keep their commit_id+count+version tuple but no domain
359+
// knowledge leaks. migration_count + migration_status unchanged.
352360
return c.JSON(fiber.Map{
353361
"ok": true,
354362
"service": "instant.dev",
355363
"commit_id": buildinfo.GitSHA,
356364
"build_time": buildinfo.BuildTime,
357365
"version": buildinfo.Version,
358-
"migration_version": mstate.Filename,
366+
"migration_version": mstate.PublicVersion(),
359367
"migration_count": mstate.Count,
360368
"migration_status": mstate.Status,
361369
})

0 commit comments

Comments
 (0)