N-11 · Lock down the account-linking SNS topic (phases 1 and 2)

[kiwifellows]

What and why

teemops-sns accepted sns:Publish from any AWS principal on the internet — the one inbound path into a TOPS install. #103 documented it and deliberately deferred the fix; this is the fix, both phases.

Closes #100 and #101. Roadmap N-11.

The topic policy is unchanged, on purpose. sns:Publish supports no message-content condition keys, and CloudFormation's custom-resource publish carries no message attributes at all — so the commented-out sns:MessageAttributes.* condition was dead on arrival twice over. It is deleted, with the reasoning left in its place so it is not revived. Narrowing happens on the subscription and in the consumer instead.

User story

Not a feature. Security work, tracked as N-11 in docs/roadmap.md, with the research in docs/features/sns-topic-publish-authorization.md.

Phase 1 — the consumer stops trusting the message body (ProcessSqsMessages)

• StackId (set by CloudFormation) cross-checked against the account in TopsRoleArn (set by the publisher)
• Only pending records are linkable — a replayed Create can no longer repoint a live account
• The pending window expires (TOPS_ACCOUNT_LINK_WINDOW_HOURS, default 24h, 0 disables)
• SNS signature verified on the SQS path, reusing SnsSignatureVerifier via a new verifyPayload()
• Alarmable rejection counters, read with php artisan aws:link-rejections

Phase 2 — install-scoped filter secret

• TopsInstallId minted once by the installer, never regenerated, persisted to generated/teemops.env
• Subscription filter policy on MessageBody, accepting a list so the value can be rotated
• Quarantine subscription with the inverse filter, so filtered-out traffic is visible instead of vanishing
• NoEcho parameter threaded through both child templates and the quick-create URL

Stated honestly, as the doc does: this is a speed bump, not an authentication boundary. The value is shared with every onboarded account admin, travels in a URL query string, and is readable via cloudformation:DescribeStacks. It raises the bar from anyone on the internet to anyone ever given an onboarding link. Phase 1 is what actually validates a link request.

How it was tested

709 tests passing (3 skipped), 46 installer assertions, scan:validate-rules clean.

• ProcessSqsMessagesTest — 8 → 20 tests, one per rejection path
• ShowLinkRejectionsTest — 9 new
• tests/install-messaging.test.sh — mint-once/reuse/rotation, and that the id reaches the stack and both templates
• All three CFN templates pass aws cloudformation validate-template

Two existing test fixtures had mismatched StackId and TopsRoleArn accounts — they were encoding the vulnerability as expected behaviour, and are corrected.

Verified end to end on a real AWS account (848310106659 / us-west-2), which is the part no test could cover — what was unknown was AWS's own behaviour:

• Correct install id → linked exactly as before
• Wrong install id (tops-vendor-audit-failtest) → filtered out of teemops_main, landed in quarantine, never linked
• Re-running the AWS step → install id unchanged (the one that silently destroys an install if it regresses)

Two things the live run caught that tests did not

1. aws:link-rejections gave a falsely reassuring answer. With a message sitting in quarantine, it reported "No account-linking messages have been rejected." Both numbers were individually correct — the cache counters only see messages that reached the poller, and a filtered message never does — but the command an operator is told to run was reassuring them about exactly the failure the quarantine queue exists to expose. The silent failure had moved one layer out rather than being closed. It now reports quarantine depth alongside the counters, gives the all-clear only when both are positively zero, and reports an unreadable queue as unknown rather than zero.

2. Reuse + expiry combined into a trap. init keeps one pending row per organization and reuses it, so measuring the window from created_at would have made expiry permanent — an org that abandoned onboarding and came back would get a fresh-looking link backed by a stale row, and every retry would reuse that same row and be rejected. Measured from updated_at instead, and issuing a link touches the record. Both halves were individually correct, which is why it would have shipped.

Beyond the written acceptance criteria

Two additions, because closing the gaps exactly as specified would have left the same hole one step to the side:

• The StackId cross-check applies to Update as well as Create — Update repoints a live account's role
• A queue message with no SNS envelope is rejected rather than processed as a "direct message" — that fallback was an unsigned path straight past the signature check

---

Quality gate

• Acceptance criteria are met — all of N-11 phase 1 · Validate account-linking messages on the consumer side #100 and N-11 phase 2 · Scope the SNS topic to this install with a filter secret #101, including the two requiring a live account
• Solves the user problem simply — the decision framework was applied
• Follows the practices in docs/practices/
• Tests written and passing
• Documentation updated — feature doc and roadmap N-11 both reflect the verified state
• No obvious performance issues
• Ready for production

Practices check

• Multi-tenancy — untouched; no new tenant queries or endpoints. The lookup remains unique_id + external_id, now with strictly more checks
• Security — no secrets committed (TOPS_INSTALL_ID blank in .env.example, NoEcho on both templates, no template defaults); the install id is never written to a log line; no raw SQL
• Simplicity — no speculative features. The one judgement call is ShowLinkRejections: a write-only counter is the silent-failure anti-pattern CLAUDE.md warns about, so the criterion is not met without a readout. If you disagree, it is the cleanest thing to delete
• Database — no migrations
• Scan definitions — scan:validate-rules passes (untouched)

Deployment note for existing installs

A child stack created before this filter existed carries no TopsInstallId, so its Delete ping is filtered out and the stack hangs until CloudFormation's ~1hr custom-resource timeout. Either delete such stacks before deploying the new SNS stack, or use --retain-resources TopsCustomNotifier. Documented in the feature doc.

generated/teemops.env must now be backed up — the install id is not recoverable from AWS, and regenerating it invalidates every issued onboarding link and silently filters every linked account's Delete ping.

Out of scope

• #102 — the audit template defaulting to the vendor's AWS account. It gains the TopsInstallId parameter here for parity, but its ParentAWSAccountId default and the templates/sync.sh public-read question are untouched; it is labelled roadmap:next
• aws:PrincipalOrgID as an opt-in install parameter — complementary, not a replacement, and filed separately in the feature doc's sequencing
• Pinning the role name (Option 4 in the research), so the message carries no authority at all

🤖 Generated with Claude Code