Phone security: STIR/SHAKEN caller ID verification by liususan091219 · Pull Request #49 · sonichi/sutando

liususan091219 · 2026-03-30T16:37:32Z

Summary

Read StirVerstat from Twilio webhook and pass through to WebSocket handler
If caller's number matches OWNER_NUMBER but StirVerstat is not TN-Validation-Passed-A, downgrade from owner to verified-only
Blocks full computer access for potentially spoofed calls
Tested: real call from owner shows TN-Validation-Passed-A, curl tests confirm pass-through

Test plan

curl test: StirVerstat=TN-Validation-Passed-A passes through as Stream parameter
curl test: StirVerstat=TN-Validation-Passed-B passes through
curl test: missing StirVerstat omits parameter
Real call from owner: StirVerstat=TN-Validation-Passed-A, owner=true
Call from non-owner unverified number: should show verified=false, owner=false

🤖 Generated with Claude Code

Read StirVerstat from Twilio webhook and pass it through to the WebSocket handler. If a caller's number matches OWNER_NUMBER but StirVerstat is not TN-Validation-Passed-A (highest attestation), downgrade from owner to verified-only — blocking full computer access for potentially spoofed calls. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

sonichi

Clean security improvement. STIR/SHAKEN caller ID verification prevents spoofed numbers from getting owner-level access.

Key points:

Reads StirVerstat from Twilio's form data (passed through STIR/SHAKEN)
If owner number matches but StirVerstat is NOT TN-Validation-Passed-A, downgrades to verified (not owner)
Logs the downgrade for audit
Passes StirVerstat through WebSocket params for downstream use
No-op when StirVerstat is absent (graceful degradation for carriers that don't support it)

One note: STIR/SHAKEN is US-only. International calls won't have StirVerstat, so !stirVerstat path correctly falls through to number match. Good.

LGTM.

#50/#51) (#1114) Three open CodeQL `js/stack-trace-exposure` warnings on main shipped `err.message`/`String(err)` directly to HTTP clients: - `src/web-client.ts:3835` — `/paidsubscriptions/data` (LAN-reachable when HTTP_HOST=0.0.0.0) - `src/web-client.ts:3874` — `/paidsubscriptions/scan` (localhost-only, but still LAN-visible via the LocalhostOnly check failure path) - `src/vision-tools.ts:583` — `/vision/*` control endpoints (127.0.0.1-bound but flagged by CodeQL's static dataflow) The pattern was `error: e?.message || String(e)` (web-client) / `error: (err as Error)?.message ?? String(err)` (vision-tools) in 6 catch sites total: 2 in web-client, 4 in vision-tools (lines 341/420/492/535 — all source the data that flowed into the line-583 respond() sink). Fix preserves diagnostic value via server-side `console.error(...)` of the full error, returns a short generic label (`'failed to read subscriptions'`, `'submitFrame failed'`, etc.) to the response body. Callers (Gemini tool consumers, the /paidsubscriptions UI) already branch on status code + ok-flag — they don't depend on the specific error text. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Chi Wang <4250911+sonichi@users.noreply.github.com>

closes #1119) (#1121) PR #1114 fixed 6 catch sites flagged by CodeQL `js/stack-trace-exposure` warnings #49 / #50 / #51. Lucy's post-merge audit caught 3 sibling sites with the same exposure pattern that CodeQL didn't flag (different upstream flow-analysis paths). Same surgical pattern as #1114: server-side `console.error(...)` + generic body label. ## Sites changed | Line | Endpoint | Pattern | |---|---|---| | 3744 | `/note-viewing` (POST) | `e instanceof Error ? e.message : 'parse failed'` → generic `'note-viewing parse failed'` | | 3806 | overlay control proxy | `'overlay control proxy failed: ' + e.message` → generic `'overlay control proxy failed'` | | 3823 | `/paidsubscriptions` (HTML) | `'Error reading subscriptions: ' + (e?.message || String(e))` → generic `'Error reading subscriptions'` | Each adds a `console.error('[web-client] /endpoint failed:', e)` server-side log so diagnostic info is preserved. ## Why these and not others Audited the file for `e.message`/`err.message` patterns; remaining occurrences (lines 1492, 1704, 2119, 2799, 3205, 3456, 3474) are all client-side JavaScript (browser-side dbg / status text) or stored in private state (`_parse_error` in the local data object, never sent in a response). Only the 3 above leaked through `res.end(...)` to a network response. ## Closes - Closes #1119 ## Test plan - [x] `npx tsc --noEmit -p .` clean - [ ] CodeQL re-scan after merge should report no new warnings for these sites (they weren't flagged before either, but the scrub closes them defensively) - [ ] Manual: trigger each endpoint failure → confirm response body is generic + full error in stderr 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Read StirVerstat from Twilio webhook and pass it through to the WebSocket handler. If a caller's number matches OWNER_NUMBER but StirVerstat is not TN-Validation-Passed-A (highest attestation), downgrade from owner to verified-only — blocking full computer access for potentially spoofed calls. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…onichi#49/sonichi#50/sonichi#51) (sonichi#1114) Three open CodeQL `js/stack-trace-exposure` warnings on main shipped `err.message`/`String(err)` directly to HTTP clients: - `src/web-client.ts:3835` — `/paidsubscriptions/data` (LAN-reachable when HTTP_HOST=0.0.0.0) - `src/web-client.ts:3874` — `/paidsubscriptions/scan` (localhost-only, but still LAN-visible via the LocalhostOnly check failure path) - `src/vision-tools.ts:583` — `/vision/*` control endpoints (127.0.0.1-bound but flagged by CodeQL's static dataflow) The pattern was `error: e?.message || String(e)` (web-client) / `error: (err as Error)?.message ?? String(err)` (vision-tools) in 6 catch sites total: 2 in web-client, 4 in vision-tools (lines 341/420/492/535 — all source the data that flowed into the line-583 respond() sink). Fix preserves diagnostic value via server-side `console.error(...)` of the full error, returns a short generic label (`'failed to read subscriptions'`, `'submitFrame failed'`, etc.) to the response body. Callers (Gemini tool consumers, the /paidsubscriptions UI) already branch on status code + ok-flag — they don't depend on the specific error text. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Chi Wang <4250911+sonichi@users.noreply.github.com>

closes sonichi#1119) (sonichi#1121) PR sonichi#1114 fixed 6 catch sites flagged by CodeQL `js/stack-trace-exposure` warnings sonichi#49 / sonichi#50 / sonichi#51. Lucy's post-merge audit caught 3 sibling sites with the same exposure pattern that CodeQL didn't flag (different upstream flow-analysis paths). Same surgical pattern as sonichi#1114: server-side `console.error(...)` + generic body label. ## Sites changed | Line | Endpoint | Pattern | |---|---|---| | 3744 | `/note-viewing` (POST) | `e instanceof Error ? e.message : 'parse failed'` → generic `'note-viewing parse failed'` | | 3806 | overlay control proxy | `'overlay control proxy failed: ' + e.message` → generic `'overlay control proxy failed'` | | 3823 | `/paidsubscriptions` (HTML) | `'Error reading subscriptions: ' + (e?.message || String(e))` → generic `'Error reading subscriptions'` | Each adds a `console.error('[web-client] /endpoint failed:', e)` server-side log so diagnostic info is preserved. ## Why these and not others Audited the file for `e.message`/`err.message` patterns; remaining occurrences (lines 1492, 1704, 2119, 2799, 3205, 3456, 3474) are all client-side JavaScript (browser-side dbg / status text) or stored in private state (`_parse_error` in the local data object, never sent in a response). Only the 3 above leaked through `res.end(...)` to a network response. ## Closes - Closes sonichi#1119 ## Test plan - [x] `npx tsc --noEmit -p .` clean - [ ] CodeQL re-scan after merge should report no new warnings for these sites (they weren't flagged before either, but the scrub closes them defensively) - [ ] Manual: trigger each endpoint failure → confirm response body is generic + full error in stderr 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…t-scope's docstring My own grep for leftover provenance used a #49[0-9][0-9] pattern, which never matched #4502 -- missed by Pro-dev's independent check, not mine. The pre-existing #4502/#4503 reference at the top of the file (predating this PR) is untouched; only the one this PR itself added to run()'s docstring is trimmed. Stand: Echo Act IV Pro Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…s cleared, not trusted (#4976) * fix(watcher): a foreign SUTANDO_INSTANCE_ID on the core's own inbox is cleared, not trusted A --force-restart (or any start) issued from inside another instance's own session inherits that instance's SUTANDO_INSTANCE_ID, even when --inbox explicitly names the core's own canonical <ws>/tasks. The watcher then watches the right directory while believing itself to be that OTHER worker's own inbox-watcher (#4502's rule: a worker's own inbox IS the routing decision) -- it never loads the core's task-event-handler config and silently stops dispatching anything it sees, with no error anywhere. Observed in production 2026-09-30, twice: pid 1184 (18:46) and its replacement pid 66624 (19:16), both with argv correctly reading `--role session --inbox <core's workspace>/tasks` but env carrying a different worker's SUTANDO_INSTANCE_ID, inherited from the shell that force-restarted them. Fix is deliberately narrow: only clears an explicitly-set SUTANDO_INSTANCE_ID when the inbox being served is the core's own canonical tasks/ dir (the same basename=="tasks" signal the existing identity-match gate already uses). It never touches a <ws>/deliveries/<id> inbox -- there, the pre-existing identity-match check already decides whether a mismatched starter must yield, and correcting the env there would make a wrong starter's identity agree with the inbox by fiat, defeating that check (this was my first, too-broad attempt; tests/watch-tasks-stream-unready-holder-restamp.test.sh scenario (d) and tests/watch-tasks-stream-inbox-and-workspace-env.test.py caught it). It also never invents an identity when SUTANDO_INSTANCE_ID was never set, since several tests legitimately run a core against a deliveries/-shaped inbox with no ambient identity at all. tests/watch-tasks-stream-config-hot-reload.test.py's worker case previously set SUTANDO_INSTANCE_ID="worker-1" while watching the bare <ws>/tasks dir -- a combination that cannot occur in production. Updated it to a realistic <ws>/deliveries/worker-1 inbox, which is what makes it a test of "a worker", not of "SUTANDO_INSTANCE_ID happens to be set". New tests/watch-tasks-stream-foreign-instance-id-corrected.test.py pins the fix directly: a foreign SUTANDO_INSTANCE_ID on the core's own inbox is corrected (an already-present handler still runs, and the correction is logged); control run against unmodified main fails cleanly, reproducing the incident exactly (TASK_FILE emitted, handler never runs). Full watch-tasks-stream*/watcher-* suite (29 files) re-run green except the pre-existing watch-tasks-stream-fallback-receipt-scope flake (confirmed failing identically on unmodified main; tracked at #4645) -- unrelated. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * style: trim comment blocks to the repo's 2-line cap review-checks' prose-cap caught three blocks over the cap (two in the hot-reload test, one in the new regression test) and the main fix's own .sh comment, which the Python-only scanner doesn't cover but the same policy applies to. The narrative moved to the PR body; code comments keep only the constraint and a pointer. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(test): realistic deliveries/<id> inbox for the fallback-receipt-scope worker cases Same gap as the hot-reload test fix: three of this file's six properties set SUTANDO_INSTANCE_ID="worker-1" while watching the bare <ws>/tasks dir — a combination that cannot occur in production and that the watcher's new foreign-identity correction (this branch's main fix) now treats as a mismatch on the core's own inbox, clearing it and breaking all three "a worker bypasses unconditionally" assertions. Caught by CI on this PR, not locally: my earlier local re-run of this file only surfaced the one already-known flake (#4645) in a truncated tail of the output and I didn't look past it — the full output showed 2/6 passed, not 5/6, meaning three genuine regressions were sitting unnoticed. CI's full log is what caught it. Fix: `run()` now takes the inbox shape from `watcher_instance` itself — <ws>/deliveries/<id> when set, <ws>/tasks when not — matching production and the hot-reload test's same fix. Back to 5/6 passed, identical to unmodified main (the one remaining failure is #4645, confirmed unrelated). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(watcher): clear the FULL inherited worker routing env, by realpath not basename kewei's PR review on this branch's prior head found two blockers: 1. Clearing only SUTANDO_INSTANCE_ID left SUTANDO_INBOX_KIND/ SUTANDO_INBOX_RESOLVER/SUTANDO_INBOX_RESOLVER_TIMEOUT/ SUTANDO_POOL_DELIVERY_SCRIPT behind -- a worker-shaped SUTANDO_INBOX_RESOLVER still runs on every entry at the core's inbox, rejects every plain task body (it expects a worker sentinel), and dispatch_task's 3-attempt retry loop gives up silently. Now the whole inherited worker-routing block is cleared together. 2. The canonical-inbox check compared basenames ("tasks" == "tasks"), which can misclassify a worker's own inbox if its path happens to end in .../tasks. Now it compares the inbox's realpath against <resolved workspace>/tasks, matching tasks-dir-resolve.sh's own notion of the core's canonical inbox. tests/watch-tasks-stream-foreign-instance-id-corrected.test.py grows two properties for these: (3) a full inherited worker env at the core's canonical inbox (incl. a real SUTANDO_INBOX_RESOLVER) still dispatches; (4)/(4b) a worker's own inbox whose basename happens to be "tasks" keeps its identity. 5/5 pass. Stand: Echo Act IV Pro Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(watcher): kewei's 2nd review — realpath in the re-stamp path too, fix a weak test assertion Two more findings from kewei's formal review at f31f10e: 1. The unready-holder re-stamp path (around line 215) still decided "empty instance id means core" by `basename(inbox) == "tasks"`, even though the dispatch-time check a few lines above it was already switched to a realpath comparison. A `<ws>/tasks` symlinked to a physically-named directory (e.g. `physical-inbox`) has `basename(TASKS_DIR_ABS) == "physical-inbox"`, so the re-stamp silently never fired for a holder on a symlinked core inbox. Now reuses the same `CANONICAL_CORE_TASKS_DIR` realpath comparison already computed earlier in the file. New case (e) in tests/watch-tasks-stream-unready-holder-restamp.test.sh pins this: a symlinked core inbox re-stamps exactly like a physical one — confirmed to fail on the basename-only version and pass on the fix. 2. tests/watch-tasks-stream-foreign-instance-id-corrected.test.py property (3) asserted `"handle" in log`, but the log is shared across properties and property (1) already wrote "handle" to it — so (3) passed trivially even when its own task (task-full-env.txt) was never dispatched. The test handler now records which task file it actually ran for, and (1)/(3) each assert their own task by name. Confirmed: at the pre-Blocker-1-fix head (68e63b6) this version of (3) now correctly FAILS (it previously passed spuriously). 5/5 on tests/watch-tasks-stream-foreign-instance-id-corrected.test.py, all green on tests/watch-tasks-stream-unready-holder-restamp.test.sh. Stand: Echo Act IV Pro Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(watcher): kewei's 3rd review — don't gate the re-stamp arm on inbox location; trim comment provenance Blocker 1: the re-stamp arm's "empty SUTANDO_INSTANCE_ID means core" check was changed in the prior commit to also require the inbox to equal the canonical <workspace>/tasks by realpath. That regressed a SUPPORTED configuration: a core watching a custom inbox with its own SUTANDO_WORKSPACE_DIR and no instance id (pinned by tests/watch-tasks-stream-inbox-and-workspace-env.test.py). The re-stamp arm's question is "is this seat the core?", which an empty instance id already answers on its own -- the inbox-location check belongs only to the earlier env-clearing block (deciding whether an INHERITED id is foreign), not here, where `sentinel-names-pid` already scopes the write to this seat's own sentinel. Dropped the inbox condition from the empty-id arm entirely. New case (f) in watch-tasks-stream-unready-holder-restamp.test.sh pins a custom inbox + separate workspace re-stamping correctly; confirmed to fail against the prior commit's realpath-gated version and pass on the fix. Blocker 3: trimmed comment references to the reviewer, review round, and issue numbers in tests/watch-tasks-stream-{config-hot-reload, foreign-instance-id-corrected,unready-holder-restamp}, keeping only the behavioral constraint each comment exists to state. Updated the foreign-instance-id-corrected module docstring, which still described two properties, to describe all five. Blocker 2 (the shipped-service live witness) is being run separately against this head once pushed. 5/5 on the corrected-id test, all green (including new case (f)) on the re-stamp test. Stand: Echo Act IV Pro Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * style(test): trim remaining provenance from the foreign-instance-id-corrected docstring The module docstring still carried an issue number, a date and a pid in its module docstring, matching exactly what AGENTS.md's comment rule forbids. Kept the behavioral statement (a worker's own inbox is the routing decision), dropped the issue number, date and pid. Stand: Echo Act IV Pro Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * style(test): trim the issue-number reference added to fallback-receipt-scope's docstring My own grep for leftover provenance used a #49[0-9][0-9] pattern, which never matched #4502 -- missed by Pro-dev's independent check, not mine. The pre-existing #4502/#4503 reference at the top of the file (predating this PR) is untouched; only the one this PR itself added to run()'s docstring is trimmed. Stand: Echo Act IV Pro Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * style(test): condense case (f)'s comment to AGENTS.md's 2-line cap The prose-cap scanner only covers Python, so this .sh comment (4 lines) slipped through CI green while still violating AGENTS.md's comment rule. Kewei caught it by eye. Condensed to 2 lines, same constraint kept (empty-id-alone for a custom inbox + separate workspace). src/watch-tasks-stream.sh is untouched (blob 8549837, unchanged since 09df259) -- test comment + PR body only, no source change, so the shipped-service live witness need not be repeated. Stand: Echo Act IV Pro Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

liususan091219 requested a review from sonichi March 30, 2026 16:38

sonichi approved these changes Mar 30, 2026

View reviewed changes

sonichi merged commit 91d863b into main Mar 30, 2026

sonichi deleted the feature/stir-shaken-verification branch March 30, 2026 16:52

This was referenced May 24, 2026

fix(security): scrub stack traces from HTTP error responses (CodeQL #49/#50/#51) #1092

Closed

[replaces #1092] fix(security): scrub stack traces from HTTP error responses (CodeQL #49/#50/#51) #1114

Merged

This was referenced May 25, 2026

fix(security): follow-up — scrub e.message from 3 more web-client.ts res.end sites that #1114 missed #1119

Closed

fix(security): scrub e.message from 3 more web-client.ts res.end sites (closes #1119) #1121

Merged

akag2 mentioned this pull request Oct 7, 2026

Desktop: rooms already read show the unread dot again after restarting the app #5183

Open

yixuan-ag2 mentioned this pull request Oct 10, 2026

feat(voice): actor mode, with upstream recovery owned by bodhi 0.4.4 #5308

Merged

Navigation Menu

Repository navigation

Phone security: STIR/SHAKEN caller ID verification - #49

Phone security: STIR/SHAKEN caller ID verification#49
sonichi merged 1 commit into
mainfrom
feature/stir-shaken-verification

liususan091219 commented Mar 30, 2026

Uh oh!

sonichi left a comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

liususan091219 commented Mar 30, 2026

Summary

Test plan

Uh oh!

sonichi left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants