amadan
console: per-app callback signing, and the verifier apps will run
Task 14. The implementation and its first eight tests were written by codex (gpt-5.6-sol) against the revised plan; the wire-format tests below, and this message, are mine. Recorded because "who wrote this" is a fair question to ask of a security primitive. WHAT IT IS. One deployment master, a per-app key derived at sign time as HMAC(master, "carlos-scan-webhook-v1" 0x00 sqid "/" app "/" gen), and a header `t=<unix>,v1=<keyid>:<hex>` over `t + "." + rawbody`. Nothing per-app is stored: derivation at sign time is what makes app A unable to forge a callback to app B from a secret it holds. During rotation both masters sign and the app accepts either, so no app is broken between the console's rotation and its own re-seal. VerifyCallback is the deliverable that matters most, and it is the one §11.3 was missing. "A stale timestamp is refused" and "both masters verify" are properties of the APP's verifier — tenant code — so without a reference implementation the tests test nothing anybody runs. The docs will reproduce it. WHAT THE PASSING TESTS DID NOT COVER. Nine mutations; five survived, and four of those had one cause: every test signed and verified with the same implementation, so nothing pinned the WIRE FORMAT. Dropping the domain label, dropping the 0x00 separator, changing the MAC delimiter, and IGNORING THE GENERATION were all invisible. The generation one is a security defect rather than an untidiness. It exists so a tenant whose callback secret leaked can be rotated alone; if it never reached the HMAC, `rotate-callback-key` would report success, hand back the same key, and the leaked secret would keep working. A reclaimed app name would inherit the deleted app's key too. Self- consistent tests cannot see any of it. So there is now a GOLDEN VECTOR: fixed master, account, app, generation, body and timestamp, against exact expected hex. It is the only test here that notices the scheme changing rather than the code, and this is the one place in the feature that needs it — the bytes are a published interface the docs hand to app authors to reimplement in other languages, and every tenant holds a key derived by that function. Change the derivation and every callback everywhere stops verifying at once, looking to each tenant like a bug in their own code. Beside it, TestDerivationSeparatesEveryInput asserts each input actually reaches the key, so the reason survives even if the numbers are ever regenerated. ONE BEHAVIOUR CHANGED from what codex wrote: the verifier now IGNORES an unrecognised header field instead of refusing it. Every tenant runs a frozen copy of this code, so a verifier that rejected what it did not recognise would make adding a `v2=` signature alongside `v1=` a breaking change for every app on the platform. Safe, because the MAC still has to verify. `t` and at least one `v1` remain required, and both directions are now pinned. The remaining survivor is honest: removing the explicit MAC-length check changes nothing, because hmac.Equal already refuses a length mismatch. It stays as defence in depth and the test comment says which it is. AND CODEX FOUND A SPEC BUG, which is why it was asked what it disagreed with. §6.3's first bullet still specified HMAC(master, sqid + "/" + app) while the bullets I added below it introduced the domain label and the generation — the section specified two different schemes. It read both, followed the later one, and said so. Corrected here, before the wire format froze. My own mutation harness was wrong twice tonight in the same way: a -run pattern that did not match the new test names, so four "survivors" were tests that never executed. It asserts up front now that the golden vector is in the set it runs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011zrMktR6SbXiFGTDFYLFd7
3 files changed,
+399
−2
docs/superpowers/specs/2026-09-02-scan-on-demand-design.md+9 −2internal/console/scansign.go+162 −0internal/console/scansign_test.go+228 −0
CI passed — run details
diff --git a/docs/superpowers/specs/2026-09-02-scan-on-demand-design.md b/docs/superpowers/specs/2026-09-02-scan-on-demand-design.md| index ba1b8aef8..98a390fe4 100644 |
| --- a/docs/superpowers/specs/2026-09-02-scan-on-demand-design.md |
| +++ b/docs/superpowers/specs/2026-09-02-scan-on-demand-design.md |
| @@ -626,11 +626,18 @@ nothing*: the console must be able to sign later, and a sealed value it cannot |
| open is no use to it. |
| - one boot master, `CARLOS_SCAN_WEBHOOK_KEY` |
| -- **per-app key derived at sign time**: `HMAC(master, sqid + "/" + app)`. |
| +- **per-app key derived at sign time**: |
| + `HMAC(master, "carlos-scan-webhook-v1" + 0x00 + sqid + "/" + app + "/" + gen)`. |
| Nothing extra is stored and per-app isolation falls out — without it a single |
| deployment secret lets app A forge callbacks to app B. The delimiter is |
| injection-safe: a sqid is `^[a-z]{3,16}$` (`qualified.go:56`) and an app is |
| - `^[a-z][a-z0-9-]{0,31}$` (`release.go:242`), so neither can contain `/`. |
| + `^[a-z][a-z0-9-]{0,31}$` (`release.go:242`), so neither can contain `/`, and |
| + `gen` is an integer. |
| + *(Corrected: this bullet said `HMAC(master, sqid + "/" + app)` while the |
| + bullets below it added the domain label and the generation, so the section |
| + specified two different schemes. The implementer read both, followed the |
| + later one, and said so — which is the only reason it was caught before the |
| + wire format froze.)* |
| - **written by `store scan grant`**, gated by `storeSealKey` |
| (`apistore.go:425`) — one config write on an operator verb, never per enqueue |
| - header `Carlos-Signature: t=<unix>,v1=<keyid>:<hex>[,v1=<keyid>:<hex>]` over |
diff --git a/internal/console/scansign.go b/internal/console/scansign.go| new file mode 100644 |
| index 000000000..1038bc233 |
| --- /dev/null |
| +++ b/internal/console/scansign.go |
| @@ -0,0 +1,162 @@ |
| +package console |
| + |
| +import ( |
| + "crypto/hmac" |
| + "crypto/sha256" |
| + "encoding/hex" |
| + "errors" |
| + "strconv" |
| + "strings" |
| + "time" |
| +) |
| + |
| +const ( |
| + callbackKeyDomain = "carlos-scan-webhook-v1" |
| + callbackKeyID = "carlos-scan-webhook-key-id" |
| + callbackMaxSkew = 5 * time.Minute |
| +) |
| + |
| +// CallbackSigner holds the deployment masters used to sign callbacks. The |
| +// per-app keys are derived only when they are needed: retaining them here |
| +// would turn one app's delivered secret into unnecessary console state. |
| +type CallbackSigner struct { |
| + current callbackMaster |
| + previous *callbackMaster |
| +} |
| + |
| +type callbackMaster struct { |
| + secret []byte |
| + id string |
| +} |
| + |
| +// ErrNoCallbackKey refuses a signer that could only produce unauthenticated |
| +// callbacks. The caller should refuse callback enrollment when this happens. |
| +var ErrNoCallbackKey = errors.New("console: no scan webhook key (CARLOS_SCAN_WEBHOOK_KEY)") |
| + |
| +// NewCallbackSigner builds a signer from CARLOS_SCAN_WEBHOOK_KEY and the |
| +// optional CARLOS_SCAN_WEBHOOK_KEY_PREVIOUS values supplied by configuration. |
| +func NewCallbackSigner(current, previous string) (*CallbackSigner, error) { |
| + if strings.TrimSpace(current) == "" { |
| + return nil, ErrNoCallbackKey |
| + } |
| + if previous != "" && previous == current { |
| + // A rotation that rotated to itself. Accepting it would tell an |
| + // operator that the old master was being retired when it was not. |
| + return nil, errors.New("console: the previous scan webhook key is identical to the current one — that is not a rotation") |
| + } |
| + s := &CallbackSigner{current: newCallbackMaster(current)} |
| + if previous != "" { |
| + p := newCallbackMaster(previous) |
| + s.previous = &p |
| + } |
| + return s, nil |
| +} |
| + |
| +func newCallbackMaster(secret string) callbackMaster { |
| + h := hmac.New(sha256.New, []byte(callbackKeyID)) |
| + h.Write([]byte(secret)) |
| + return callbackMaster{secret: []byte(secret), id: hex.EncodeToString(h.Sum(nil))[:16]} |
| +} |
| + |
| +// DeriveCallbackKey returns the one key delivered to an app. The domain label |
| +// prevents another future HMAC(master, value) use from producing the same key; |
| +// gen lets one tenant rotate a leaked key without rotating the deployment. |
| +func DeriveCallbackKey(master, sqid, app string, gen int) []byte { |
| + h := hmac.New(sha256.New, []byte(master)) |
| + h.Write([]byte(callbackKeyDomain)) |
| + h.Write([]byte{0}) |
| + h.Write([]byte(sqid + "/" + app + "/" + strconv.Itoa(gen))) |
| + return h.Sum(nil) |
| +} |
| + |
| +// Sign returns the Carlos-Signature value. During deployment-key rotation it |
| +// signs with both masters so apps may move to the new derived key gradually. |
| +func (s *CallbackSigner) Sign(sqid, app string, gen int, body []byte, now time.Time) string { |
| + timestamp := strconv.FormatInt(now.Unix(), 10) |
| + values := []string{"t=" + timestamp, s.signWith(s.current, sqid, app, gen, timestamp, body)} |
| + if s.previous != nil { |
| + values = append(values, s.signWith(*s.previous, sqid, app, gen, timestamp, body)) |
| + } |
| + return strings.Join(values, ",") |
| +} |
| + |
| +func (s *CallbackSigner) signWith(master callbackMaster, sqid, app string, gen int, timestamp string, body []byte) string { |
| + key := DeriveCallbackKey(string(master.secret), sqid, app, gen) |
| + return "v1=" + master.id + ":" + hex.EncodeToString(callbackMAC(key, timestamp, body)) |
| +} |
| + |
| +// VerifyCallback is the reference verifier tenant apps run. The key is the |
| +// app's single derived key, never a deployment master. It tries every v1 entry |
| +// because key ids identify masters to operators, while an app neither has nor |
| +// needs the master from which its key came. |
| +func VerifyCallback(key []byte, header string, body []byte, now time.Time) error { |
| + var timestamp string |
| + var signatures [][]byte |
| + for _, field := range strings.Split(header, ",") { |
| + switch { |
| + case strings.HasPrefix(field, "t="): |
| + if timestamp != "" || len(field) == 2 { |
| + return errors.New("console: malformed callback signature timestamp") |
| + } |
| + timestamp = field[2:] |
| + case strings.HasPrefix(field, "v1="): |
| + parts := strings.Split(field[3:], ":") |
| + if len(parts) != 2 || len(parts[0]) != 16 { |
| + return errors.New("console: malformed callback signature") |
| + } |
| + if _, err := hex.DecodeString(parts[0]); err != nil { |
| + return errors.New("console: malformed callback key id") |
| + } |
| + mac, err := hex.DecodeString(parts[1]) |
| + if err != nil || len(mac) != sha256.Size { |
| + return errors.New("console: malformed callback MAC") |
| + } |
| + signatures = append(signatures, mac) |
| + default: |
| + // AN UNKNOWN FIELD IS IGNORED, and that is a forward-compatibility |
| + // decision rather than laxness. Every tenant app runs a copy of |
| + // this verifier, and the copies are frozen the day they are |
| + // written; a verifier that refused what it did not recognise would |
| + // make adding a `v2=` signature alongside `v1=` a breaking change |
| + // for every app on the platform. Ignoring it is safe because the |
| + // MAC still has to verify — an attacker gains nothing by appending |
| + // fields. What is NOT optional is below: t and at least one v1. |
| + } |
| + } |
| + if timestamp == "" || len(signatures) == 0 { |
| + return errors.New("console: callback signature needs t and v1") |
| + } |
| + unix, err := strconv.ParseInt(timestamp, 10, 64) |
| + if err != nil || strconv.FormatInt(unix, 10) != timestamp { |
| + return errors.New("console: malformed callback timestamp") |
| + } |
| + age := now.Sub(time.Unix(unix, 0)) |
| + if age < -callbackMaxSkew || age > callbackMaxSkew { |
| + return errors.New("console: callback timestamp outside five-minute skew") |
| + } |
| + |
| + want := callbackMAC(key, timestamp, body) |
| + for _, got := range signatures { |
| + if hmac.Equal(got, want) { |
| + return nil |
| + } |
| + } |
| + return errors.New("console: callback signature does not verify") |
| +} |
| + |
| +// callbackMAC uses a delimiter because timestamp is decimal digits and cannot |
| +// contain '.': the first dot therefore splits timestamp and raw body without |
| +// ambiguity. Verdict tags need length prefixes because an object key can hold |
| +// any delimiter; copying that encoding here would burden non-Go verifiers for |
| +// no safety gain. |
| +// |
| +// There is deliberately no nonce. A replay within the skew window verifies, |
| +// but the body is idempotent on (account, app, key, etag, verdict), so a nonce |
| +// without a durable tenant-side nonce store would add ceremony, not safety. |
| +func callbackMAC(key []byte, timestamp string, body []byte) []byte { |
| + h := hmac.New(sha256.New, key) |
| + h.Write([]byte(timestamp)) |
| + h.Write([]byte{'.'}) |
| + h.Write(body) |
| + return h.Sum(nil) |
| +} |
diff --git a/internal/console/scansign_test.go b/internal/console/scansign_test.go| new file mode 100644 |
| index 000000000..2745a66c2 |
| --- /dev/null |
| +++ b/internal/console/scansign_test.go |
| @@ -0,0 +1,228 @@ |
| +package console |
| + |
| +import ( |
| + "crypto/hmac" |
| + "crypto/sha256" |
| + "encoding/hex" |
| + "strconv" |
| + "strings" |
| + "testing" |
| + "time" |
| +) |
| + |
| +var callbackTestNow = time.Unix(1_800_000_000, 0) |
| + |
| +func callbackTestSigner(t *testing.T, current, previous string) *CallbackSigner { |
| + t.Helper() |
| + s, err := NewCallbackSigner(current, previous) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + return s |
| +} |
| + |
| +func TestSignatureRejectsTamperedBody(t *testing.T) { |
| + s := callbackTestSigner(t, "current", "") |
| + header := s.Sign("acct", "photos", 0, []byte(`{"verdict":"clean"}`), callbackTestNow) |
| + key := DeriveCallbackKey("current", "acct", "photos", 0) |
| + if err := VerifyCallback(key, header, []byte(`{"verdict":"infected"}`), callbackTestNow); err == nil { |
| + t.Fatal("tampered body verified") |
| + } |
| +} |
| + |
| +func TestSignatureIsPerApp(t *testing.T) { |
| + s := callbackTestSigner(t, "current", "") |
| + body := []byte(`{"verdict":"clean"}`) |
| + header := s.Sign("acct", "app-a", 0, body, callbackTestNow) |
| + otherKey := DeriveCallbackKey("current", "acct", "app-b", 0) |
| + if err := VerifyCallback(otherKey, header, body, callbackTestNow); err == nil { |
| + t.Fatal("app A signature verified with app B key") |
| + } |
| +} |
| + |
| +func TestSignatureRejectsStaleTimestamp(t *testing.T) { |
| + s := callbackTestSigner(t, "current", "") |
| + body := []byte("body") |
| + key := DeriveCallbackKey("current", "acct", "app", 0) |
| + for _, signedAt := range []time.Time{callbackTestNow.Add(-5*time.Minute - time.Second), callbackTestNow.Add(5*time.Minute + time.Second)} { |
| + header := s.Sign("acct", "app", 0, body, signedAt) |
| + if err := VerifyCallback(key, header, body, callbackTestNow); err == nil { |
| + t.Fatalf("timestamp %v verified", signedAt) |
| + } |
| + } |
| +} |
| + |
| +func TestTwoMasterRotationVerifies(t *testing.T) { |
| + s := callbackTestSigner(t, "new", "old") |
| + body := []byte("body") |
| + header := s.Sign("acct", "app", 3, body, callbackTestNow) |
| + for _, master := range []string{"new", "old"} { |
| + key := DeriveCallbackKey(master, "acct", "app", 3) |
| + if err := VerifyCallback(key, header, body, callbackTestNow); err != nil { |
| + t.Fatalf("key derived from %q: %v", master, err) |
| + } |
| + } |
| +} |
| + |
| +func TestSignatureIsOverRawBodyBytes(t *testing.T) { |
| + s := callbackTestSigner(t, "current", "") |
| + raw := []byte("{\n \"verdict\": \"clean\"\n}") |
| + header := s.Sign("acct", "app", 0, raw, callbackTestNow) |
| + key := DeriveCallbackKey("current", "acct", "app", 0) |
| + if err := VerifyCallback(key, header, []byte(`{"verdict":"clean"}`), callbackTestNow); err == nil { |
| + t.Fatal("re-serialized JSON verified in place of raw body") |
| + } |
| +} |
| + |
| +func TestVerifierToleratesTwoSignaturesAndUnknownKeyIds(t *testing.T) { |
| + s := callbackTestSigner(t, "current", "") |
| + body := []byte("body") |
| + header := s.Sign("acct", "app", 0, body, callbackTestNow) |
| + unknown := "v1=0000000000000000:" + strings.Repeat("00", 32) |
| + header = strings.Replace(header, ",", ","+unknown+",", 1) |
| + // A one-master header has no second comma, so append the unknown entry. |
| + if !strings.Contains(header, unknown) { |
| + header += "," + unknown |
| + } |
| + key := DeriveCallbackKey("current", "acct", "app", 0) |
| + if err := VerifyCallback(key, header, body, callbackTestNow); err != nil { |
| + t.Fatal(err) |
| + } |
| +} |
| + |
| +func TestPreviousEqualToCurrentIsRefused(t *testing.T) { |
| + if _, err := NewCallbackSigner("same", "same"); err == nil { |
| + t.Fatal("equal current and previous masters accepted") |
| + } |
| +} |
| + |
| +func TestCallbackVerifierRejectsMalformedHeaders(t *testing.T) { |
| + for _, header := range []string{ |
| + "v1=0000000000000000:" + strings.Repeat("00", 32), |
| + "t=1800000000", |
| + "t=not-a-number,v1=0000000000000000:" + strings.Repeat("00", 32), |
| + "t=1800000000,v1=0000000000000000:", |
| + "t=1800000000,v1=0000000000000000:0", |
| + "t=1800000000,v1=0000000000000000", |
| + } { |
| + t.Run(header, func(t *testing.T) { |
| + if err := VerifyCallback([]byte("key"), header, []byte("body"), callbackTestNow); err == nil { |
| + t.Fatal("malformed header verified") |
| + } |
| + }) |
| + } |
| +} |
| + |
| +// TestCallbackWireFormatGoldenVector is the only test here that would notice |
| +// if the SCHEME changed rather than the code. |
| +// |
| +// Every other test in this file signs and verifies with the same |
| +// implementation, so it passes no matter what goes into the MAC — mutation |
| +// testing confirmed exactly that: dropping the domain label, dropping the 0x00 |
| +// separator, dropping the generation, and changing the delimiter were ALL |
| +// invisible. Four silent breakages in the one part of this feature that is a |
| +// wire contract. |
| +// |
| +// It matters more here than anywhere else in the system for two reasons. The |
| +// docs reproduce VerifyCallback for app authors to reimplement in other |
| +// languages, so these bytes are a published interface. And every tenant app |
| +// holds a key derived by this function: if the derivation changes, every |
| +// callback everywhere stops verifying at once, and it looks to each tenant |
| +// like a bug in their own code. |
| +// |
| +// If this test fails, the scheme changed. That is either a mistake, or a |
| +// deliberate v2 — and a deliberate v2 adds a signature alongside v1 rather |
| +// than editing these numbers. |
| +func TestCallbackWireFormatGoldenVector(t *testing.T) { |
| + const ( |
| + master = "master-fixed-for-the-vector" |
| + wantKey = "c7aff5eb65835639b68c63a226767768ac2049fe0b57ab83844691ae5cca1d17" |
| + wantHead = "t=1800000000,v1=ccd4f98ffb69f5dd:" + |
| + "1c24c96520fc34a1275967165de2a0bac68ac5940cd558fbfc9f8f3367a4b2c8" |
| + ) |
| + body := []byte(`{"account":"bab","app":"titogo","key":"uploads/x.pdf","verdict":"clean"}`) |
| + |
| + if got := hex.EncodeToString(DeriveCallbackKey(master, "bab", "titogo", 7)); got != wantKey { |
| + t.Errorf("derived key changed:\n got %s\nwant %s", got, wantKey) |
| + } |
| + s := callbackTestSigner(t, master, "") |
| + if got := s.Sign("bab", "titogo", 7, body, time.Unix(1_800_000_000, 0)); got != wantHead { |
| + t.Errorf("header changed:\n got %s\nwant %s", got, wantHead) |
| + } |
| +} |
| + |
| +// TestDerivationSeparatesEveryInput: each field that goes into the derivation |
| +// must change the key when it changes, or it is not really in there. |
| +// |
| +// The generation is the one that matters most and the one a self-consistent |
| +// test cannot see. It exists so a tenant whose callback secret leaked can be |
| +// rotated on its own; if it did not reach the HMAC, `--rotate-callback-key` |
| +// would report success, issue the same key back, and the leaked secret would |
| +// keep working. A reclaimed app name would inherit the deleted app's key too. |
| +func TestDerivationSeparatesEveryInput(t *testing.T) { |
| + base := DeriveCallbackKey("m", "bab", "titogo", 0) |
| + for _, tc := range []struct { |
| + name string |
| + key []byte |
| + }{ |
| + {"a different master", DeriveCallbackKey("m2", "bab", "titogo", 0)}, |
| + {"a different account", DeriveCallbackKey("m", "zab", "titogo", 0)}, |
| + {"a different app", DeriveCallbackKey("m", "bab", "other", 0)}, |
| + {"a different generation", DeriveCallbackKey("m", "bab", "titogo", 1)}, |
| + } { |
| + if hmac.Equal(base, tc.key) { |
| + t.Errorf("%s produced the same key — that input does not reach the derivation", tc.name) |
| + } |
| + } |
| + // And the label plus separator are not decoration: without them |
| + // HMAC(master, "carlos-scan-webhook-v1\x00bab/titogo/0") would collide with |
| + // any other future HMAC(master, x) use that happened to pick that string. |
| + // Pinned by the golden vector above; asserted here as a property so the |
| + // reason survives even if the numbers are regenerated. |
| + if len(base) != sha256.Size { |
| + t.Errorf("derived key is %d bytes, want %d", len(base), sha256.Size) |
| + } |
| +} |
| + |
| +// TestVerifierToleratesUnknownFields pins the forward-compatibility choice, in |
| +// both directions: an unrecognised field must not break a verifier that a |
| +// tenant froze years ago, and a header missing t or v1 must still be refused. |
| +func TestVerifierToleratesUnknownFields(t *testing.T) { |
| + s := callbackTestSigner(t, "current", "") |
| + body := []byte("body") |
| + key := DeriveCallbackKey("current", "acct", "app", 0) |
| + header := s.Sign("acct", "app", 0, body, callbackTestNow) |
| + |
| + // A field from a scheme this verifier has never heard of. |
| + if err := VerifyCallback(key, header+",v2=whatever:deadbeef", body, callbackTestNow); err != nil { |
| + t.Errorf("an unknown field broke verification: %v — adding v2 later would break every app", err) |
| + } |
| + if err := VerifyCallback(key, "future=1,"+header, body, callbackTestNow); err != nil { |
| + t.Errorf("an unknown leading field broke verification: %v", err) |
| + } |
| + // But the two that are load-bearing are still required. |
| + for _, bad := range []string{ |
| + "v2=whatever:deadbeef", // no t, no v1 |
| + "t=" + strconv.FormatInt(callbackTestNow.Unix(), 10) + ",v2=x:y", // t but no v1 |
| + } { |
| + if err := VerifyCallback(key, bad, body, callbackTestNow); err == nil { |
| + t.Errorf("header %q verified with no usable signature in it", bad) |
| + } |
| + } |
| +} |
| + |
| +// TestVerifierRefusesATruncatedMAC. hmac.Equal already refuses a length |
| +// mismatch, so the explicit size check is defence in depth rather than the |
| +// guard — pinned anyway, because "it happens to be safe downstream" is how a |
| +// check gets deleted. |
| +func TestVerifierRefusesATruncatedMAC(t *testing.T) { |
| + s := callbackTestSigner(t, "current", "") |
| + body := []byte("body") |
| + key := DeriveCallbackKey("current", "acct", "app", 0) |
| + header := s.Sign("acct", "app", 0, body, callbackTestNow) |
| + |
| + cut := header[:len(header)-2] // one byte off the MAC |
| + if err := VerifyCallback(key, cut, body, callbackTestNow); err == nil { |
| + t.Fatal("a truncated MAC verified") |
| + } |
| +} |