From b1ddc4fab22cb5720814fdcaa318dc8214ef85c3 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sun, 20 Sep 2026 01:05:13 -0700 Subject: [PATCH 1/5] docs(security): correct the audited specs and give SELF_HOST.md a rationale MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes inaccuracies the spec audit found in the security group, and moves evidence out of the specs that had no headroom left for the corrections. Corrections - security-local.md's persisted-state FAIL IF named "three callers" of `write_file_atomically`; there are five. It now says every caller and tells the auditor to enumerate from the file. - security-audit.md's Domains FAIL IF quantified `.github/audit/` and `.vscode/` as outside *every* consumer of the workflow-audit diff window, which reads as "absent from all" and so never fires on the failure it exists to catch — widening one consumer without the others. Inverted to *any*. The rule is latent today (all four consumers derive from one WINDOW array), but it was an audited rule that checked nothing. - security-remote.md and SELF_HOST.md claimed deploy-lint is the only automated signal about the Windows installer; ps1-cmdlet-lint reads the same file. - security.md said `recovery.json` is written at the umask. It is written 0600 under a 0700 directory; the real residual is Windows, where the mode is a no-op and only standalone applies a DACL. - security.md's Persisted state and Desktop Releases rows said `audit`; both are pinned on every `pnpm test` (`cargo test`, sign-and-deploy.test.mjs). security-ci.md now names the same test. - security-local.md's Settings-Sync FAIL IF said the rule was kept by a call not existing; a test scans for it. - security-ci.md's `pull_request_target` rule now quantifies over an `on:` trigger, so the comment in hosted-preview.yml is not a false FAIL. - security-audit.md now names `BASH_DEFAULT_TIMEOUT_MS`, states the monotone-upward issue-title rule, says `claude_args` rather than `--agents`, and drops the false "disjoint scopes" claim. - SECURITY.md named five audited checklists and omitted security-hosted.md; it now points at security.md's Domains table. Same fix on the website page. - hosted.md: root `pnpm test` runs the deploy suites, and archive hashes are `verifyPackages`, not `preflight`. security-hosted.md gains its `Pinned by`. - SELF_HOST.md: `setup-password.json` added to the state tree, the stale dev-relay port claim corrected, and the Windows kill block no longer selects by image name, which its own trap forbids. Prompts - supply-chain.md now carries the `GH_TOKEN=$AUDIT_PAT` instruction its secret-scanning and Dependabot checks need; without it both read as absent. - application-security.md gains the Hosted paths security-hosted.md requires and Hosted bullets for its qualitative pass. Lint - A root-level spec now pairs with a root-level rationale, so check 15 covers SELF_HOST.md (6,036 words, previously exempt) and check 11 sees its markers. - New check 17: a `**FAIL IF**` rule leads a line only in a security spec, so an audited rule cannot be written where no domain claims it. - Both carry self-test cases. Condensation - SELF_HOST.rationale.md is new; the Invariants, mechanism-map and traps narratives move into it or point at the code comment that already holds them, and the Installer contract points at security-remote.md rather than reproducing it. - security-remote.md, security-supply-chain.md, security.md, security-local.md and security-audit.md drop restatements their rationale files or the audit prompts already carry. Budgets ratcheted down for SELF_HOST.md (6200→6050), security-remote.md (5850→5750) and security-supply-chain.md (1250→1150). Co-Authored-By: Claude Fable 5.1 --- .github/audit/application-security.md | 38 ++++++-- .github/audit/supply-chain.md | 18 ++++ SECURITY.md | 11 +-- SELF_HOST.md | 128 ++++++++++++------------- SELF_HOST.rationale.md | 105 ++++++++++++++++++++ docs/specs/hosted.md | 10 +- docs/specs/security-audit.md | 54 ++++++----- docs/specs/security-audit.rationale.md | 6 ++ docs/specs/security-ci.md | 6 +- docs/specs/security-ci.rationale.md | 2 +- docs/specs/security-hosted.md | 8 +- docs/specs/security-local.md | 12 ++- docs/specs/security-remote.md | 17 ++-- docs/specs/security-supply-chain.md | 12 +-- docs/specs/security.md | 22 ++--- hosted/src/style.css | 6 ++ scripts/spec-lint-selftest.mjs | 34 +++++-- scripts/spec-lint.mjs | 45 +++++++-- scripts/spec-word-budgets.json | 6 +- website/src/pages/SecurityDocs.tsx | 2 +- 20 files changed, 376 insertions(+), 166 deletions(-) create mode 100644 SELF_HOST.rationale.md diff --git a/.github/audit/application-security.md b/.github/audit/application-security.md index 50fb5a8a6..57710a805 100644 --- a/.github/audit/application-security.md +++ b/.github/audit/application-security.md @@ -12,10 +12,16 @@ This is a code-and-specs audit of the product's own boundaries — the remote control stack, and the local application. You need no GitHub API access and no PAT — do not use one. -For Hosted accounts, read `docs/specs/hosted.md`, `hosted/server/`, the packed -core/auth modules in `vendor/`, and `hosted/src/`. Verify the archive hashes -against `vendor/build.json`. Distinguish tested code from pending production -configuration; do not treat local provider simulations as live OAuth acceptance. +For Hosted accounts, read `docs/specs/hosted.md`, `hosted/server/`, +`hosted/src/`, `hosted/scripts/`, `hosted/wrangler.jsonc`, the packed core/auth +modules in `vendor/`, and `.github/workflows/hosted-preview.yml` and +`.github/workflows/hosted-production.yml` — `docs/specs/security-hosted.md`'s +Deployment boundary quantifies over the preview and production paths, which +live in those scripts and workflows rather than in the Worker. Verify the +archive hashes against `vendor/build.json`. Distinguish tested code from +pending production configuration; do not treat local provider simulations as +live OAuth acceptance, and treat a checked-in placeholder as no evidence about +an external control. Read, at minimum: `docs/specs/remote-security-model.md` **and its paired `docs/specs/remote-security-model.rationale.md`**, `docs/specs/relay.md`, @@ -146,6 +152,23 @@ Be adversarial, and go past the `FAIL IF` list. Ask specifically: reconstruct and relay only the registered pane-level shapes, never relay a nested document's location, and target only that origin plus the validated app origin—never a wildcard or foreign origin. +- **Is the Hosted origin the only one that can drive Hosted?** Trace a request + from `hosted/server/worker.ts` through `workerApp`'s origin gate and + `secureHeaders`: a foreign `Host`, a preview hostname, a misconfigured + deployment's error path, and the SPA fallback must each answer without + credentialed CORS, without a cacheable shell, and without inline script. + Check that authentication cookies stay `__Host-`, Secure, HttpOnly, `Path=/` + and Domain-less, and that no session token reaches browser JSON or storage. +- **Can a Hosted login become terminal access, or an account become someone + else's?** `authPolicy` must keep explicit linking and independent logins; a + callback whose initiating login was revoked must fail; an unused or unknown + provider credential must enable nothing. No Hosted endpoint may mint a Burrow + ACL grant or stand in for the encrypted pairing and presence proof. +- **Does anything from the test or preview build reach production?** The + production Worker must not export the captured-email inbox, the deterministic + clock, or the testing injection module; preview must not copy production + routes, bindings, or credentials, must not call real mail or OAuth, and its + cleanup must check out the base branch rather than the closed PR's. - Does the shipped code still match what the specs and this section claim? Spec drift is a finding; say which side is wrong. The newest sections are the ones most likely to have drifted: `remote-security-model.md`'s Presence proofs, @@ -174,9 +197,10 @@ as a subtraction rather than as two named subdirectories, which is the shape to prefer when you find the next one. Today the remainder is `lib/`, `relay/`, `remote-lib-common/`, `standalone/`, -`vscode-ext/`, `dor/`, `dor-lib-common/`, `canopy/`, `deploy/`, `docs/`, -`.impeccable/`, and the root files — but treat that as a description of the -current tree, not as your scope. Your scope is the remainder. +`vscode-ext/`, `dor/`, `dor-lib-common/`, `hosted/`, `vendor/`, `canopy/`, +`deploy/`, `docs/`, `.impeccable/`, and the root files — but treat that as a +description of the current tree, not as your scope. Your scope is the +remainder. Remote control is where the depth goes; the rest is a sweep for anything that would be a security hole in a terminal that runs local shells — command diff --git a/.github/audit/supply-chain.md b/.github/audit/supply-chain.md index c3910d42b..30d1783ff 100644 --- a/.github/audit/supply-chain.md +++ b/.github/audit/supply-chain.md @@ -6,6 +6,24 @@ **Output file:** `audit-supply-chain.md` +The default `$GH_TOKEN` in this environment is a workflow `GITHUB_TOKEN` and +does **not** have admin scope. GitHub omits `security_and_analysis` for a +non-admin token and answers 403 rather than 204 on the Dependabot alert +endpoint, so the secret-scanning and Dependabot checks read as absent when they +are on. Prefix those `gh api` calls with `GH_TOKEN=$AUDIT_PAT`: + +```sh +GH_TOKEN=$AUDIT_PAT gh api repos/$GITHUB_REPOSITORY --jq .security_and_analysis +GH_TOKEN=$AUDIT_PAT gh api repos/$GITHUB_REPOSITORY/vulnerability-alerts +``` + +`$AUDIT_PAT` is a fine-grained, read-only PAT covering Administration + +Secrets + Environments, guaranteed present by an earlier step. If a prefixed +call still returns 403, record FAIL with the note "PAT scope drifted from +docs/specs/security-audit.md". When run by `scripts/security-audit-local.sh` +without `AUDIT_PAT`, use the operator's existing `gh` authentication without a +`GH_TOKEN=` override, and report an inaccessible check as `UNVERIFIABLE`. + The workspace is installed by an earlier workflow step, so try the generate-deps check directly; if it errors on a missing module, run `pnpm install --frozen-lockfile` first. The check requires a clean working tree diff --git a/SECURITY.md b/SECURITY.md index ad3d932ef..c2de7846b 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -13,13 +13,10 @@ intend to do about it; there is no bounty. security spec, [`docs/specs/security.md`](docs/specs/security.md), published at — whole, but with the guarantees table and the two lists narrowed there to that page's audience, so the spec itself is -where every row appears together. It names the five audited -checklists beside it — [local](docs/specs/security-local.md), -[remote control](docs/specs/security-remote.md), -[supply chain](docs/specs/security-supply-chain.md), -[CI and releases](docs/specs/security-ci.md), and -[the audit itself](docs/specs/security-audit.md) — whose `FAIL IF` lines a -nightly audit executes and every VS Code release is gated on. A failure files a +where every row appears together. Its +[Domains table](docs/specs/security.md#how-the-guarantees-are-checked) names +every audited checklist beside it, whose `FAIL IF` lines a nightly audit +executes and every VS Code release is gated on. A failure files a public issue labeled [`security-audit-failure`](https://github.com/diffplug/dormouse/issues?q=is%3Aissue+label%3Asecurity-audit-failure); open ones are live, closed ones are the record. diff --git a/SELF_HOST.md b/SELF_HOST.md index 902a088a1..26cd0dbfc 100644 --- a/SELF_HOST.md +++ b/SELF_HOST.md @@ -74,7 +74,7 @@ installed release, which the installer and `manage status` both print. - **A tailnet** with MagicDNS and HTTPS certificates enabled, Tailscale running on this laptop and on the phone that will run Pocket. Whether the HTTPS origin stays private is a deployment choice, not a security premise - (`docs/specs/security-remote.md` → "Network posture"). + (`docs/specs/security-remote.md` → "Network posture (self-hosted)"). - **macOS, Windows or Linux.** Each installer refuses the others. On a fourth OS, or Linux without systemd, design the native service manager with the user rather than translating LaunchAgent, Scheduled Task or unit-file commands @@ -110,7 +110,7 @@ installed release, which the installer and `manage status` both print. Both bake it into their Node Burrow bundles, and the relay socket is in neither webview, so no webview CSP change widens the allowlist - (`docs/specs/relay.md` → "Where a Burrow may reach a Relay"). + (`docs/specs/relay.md` → "Where a Burrow may reach a Relay (self-host builds)"). ## What the installer does @@ -142,6 +142,7 @@ administrator rights: account.json burrows.json push-subscriptions.json + setup-password.json vapid.json ``` @@ -176,6 +177,8 @@ what keeps the task free of a stored password; Linux alone opts out, with - **The service is registered and running, declares the run-at-load and restart-on-exit of the [Mechanism map](#mechanism-map), and carries no credential** — a definition it cannot read at all fails rather than passes. + **No `manage verify` inspects the definition for a credential today**: the + installers put none there, and nothing checks that they still do not. Plus what only the live system shows: macOS, loaded in `gui/$UID` with a plist that lints; Windows, task `Running`, no execution time limit, restarts on failure, unelevated, unstopped by battery or idle, `bin\run-relay.ps1` still @@ -231,8 +234,9 @@ Establish with the user what the script cannot: installer installs exactly what is checked out. - **Their phone runs Tailscale** and is signed in to the same tailnet. - **Port 3100 is free.** Unchecked before installation; a stale listener blocks - the new Relay from binding and fails the post-install identity check - (`PORT=3000 pnpm dev:relay` pins the dev Relay to 3000): + the new Relay from binding and fails the post-install identity check. A dev + Relay is not normally the culprit: `pnpm dev:relay` takes any free port unless + `PORT` names one. ```sh # macOS @@ -317,12 +321,14 @@ pkill -f 'Dormouse Relay/current/relay/dist/index.js' ``` ```powershell -# Windows — run-relay.ps1's supervision loop restarts after its 10s throttle, -# so wait ~15s before reading status. -Get-CimInstance Win32_Process -Filter "Name='node.exe'" | - Where-Object { $_.CommandLine -like '*Dormouse Relay*' } | +# Windows — select by image path and command line, never image name (see the +# traps). run-relay.ps1's supervision loop restarts after its 10s throttle, so +# wait ~15s before reading status. +$root = "$env:LOCALAPPDATA\Dormouse Relay" +Get-CimInstance Win32_Process | + Where-Object { $_.ExecutablePath -like "$root\*" -or $_.CommandLine -like "*$root*" } | ForEach-Object { Stop-Process -Id $_.ProcessId -Force } -& "$env:LOCALAPPDATA\Dormouse Relay\bin\manage.cmd" status +& "$root\bin\manage.cmd" status ``` ```sh @@ -585,14 +591,11 @@ install. installed release from the current checkout; it never pulls, fetches, switches branches, or schedules an updater. -The security properties this deployment is audited against are the "Network -posture (self-hosted)" and "Credentials at rest" `FAIL IF` lines in -`docs/specs/security-remote.md`. **Those lines bind all three installers** — a control present in -one and absent from another is a finding — and `scripts/deploy-lint.mjs` -(`pnpm lint:deploy`, part of `pnpm test`) checks each one textually against each -installer, with `scripts/deploy-lint-selftest.mjs` deleting each matched control -in turn to keep that honest. On Windows, which nothing in CI executes, the lint -is the only automated signal at all. +The security properties this deployment is audited against, and everything +that enforces them, are the "Network posture (self-hosted)" and "Credentials at +rest" `FAIL IF` lines in `docs/specs/security-remote.md`. **Those lines bind all +three installers** — a control present in one and absent from another is a +finding. **Each release is self-contained**: the production Relay tree, `lib/dist-pocket`, and a copy of the exact Node binary the build ran under, so @@ -618,15 +621,12 @@ service-definition paths are under "What the installer does". | Entry | `/bin/bash bin/run-relay` | `powershell.exe -NoProfile -NonInteractive -ExecutionPolicy Bypass -File bin\run-relay.ps1`, at an absolute interpreter path | `ExecStart=/bin/bash "/bin/run-relay"` | Two rows are load-bearing Windows deviations, both because the macOS mechanism -has no unprivileged Windows equivalent. **The release pointer is a file**: no -unprivileged replaceable directory symlink exists (a junction cannot be renamed -over a junction, and delete-then-create leaves `current` naming nothing), while -a file swaps atomically with `MoveFileEx(MOVEFILE_REPLACE_EXISTING)`; the switch -still asserts the pointer advanced. **KeepAlive lives in the wrapper**: Task -Scheduler restarts a task that *fails*, not one that exits 0, so -`run-relay.ps1` is a supervision loop with the plist's own 10-second throttle, -and `manage verify` checks that loop is present rather than trusting task -settings. Linux's one deviation: **lingering is the availability knob, and it is +has no unprivileged Windows equivalent. **The release pointer is a file**, +swapped atomically, and the switch still asserts the pointer advanced +(rationale). **KeepAlive lives in the wrapper**: `run-relay.ps1` is a +supervision loop with the plist's own 10-second throttle, and `manage verify` +checks that loop is present rather than trusting task settings (rationale). +Linux's one deviation: **lingering is the availability knob, and it is opt-in** (`--linger`, Prerequisites) — a user manager stops at logout like a LaunchAgent, the installer never changes that silently, and `manage verify` reports which mode is live rather than asserting either. @@ -640,41 +640,39 @@ reports which mode is live rather than asserting either. release id fails without deleting its contents. - **State outlives code.** `config/` and `state/` sit outside `releases/`, are readable only by the installing user, and survive every update, prune and - uninstall; purging is separate and explicitly confirmed. The Relay generates - `state/setup-password.json` once from 32 CSPRNG bytes, validates it on every - boot, and never accepts an operator-supplied setup credential. - `config/relay.env` is generated once, then preserved byte-for-byte. + uninstall; purging is separate and explicitly confirmed. `config/relay.env` + is generated once, then preserved byte-for-byte. **Must read its last assignment for each key**, stripping only one matched pair of double quotes, in the installer, service wrapper, and management commands. `scripts/installer-verify-test.mjs` exercises the unix readers against the shipped wrapper parser. **A preserved file missing installer-owned keys is half-written**: name them - and stop, never rewrite values that cannot be proven stale. **`manage verify` - walks every file in `state\` on Windows**, where Node's modes are a no-op. + and stop, never rewrite values that cannot be proven stale. The setup + credential the Relay generates into `state/`, and what `manage verify` walks + on Windows, are `docs/specs/security-remote.md` → "Credentials at rest". - **The enrollment offer rotates on every run before the first Burrow enrolls**, - including updates that preserve `relay.env`; `state/burrows.json` then disables - it permanently until a state purge. **Minted last** — after release, Serve and - pruning succeed — so a failure leaves the previous offer unspent. - `run-relay` exports `DORMOUSE_ENROLL_TOKEN_FILE`; unset, the Relay refuses - every offer (`docs/specs/relay.md` → Configuration). Generation and - protection: `docs/specs/security-remote.md` → "Credentials at rest". + and is **minted last** — after release, Serve and pruning succeed — so a + failure leaves the previous offer unspent. `run-relay` exports + `DORMOUSE_ENROLL_TOKEN_FILE`; unset, the Relay refuses every offer + (`docs/specs/relay.md` → Configuration). Rotation, the `state/burrows.json` + marker that ends it, generation and protection: + `docs/specs/security-remote.md` → "Credentials at rest". - **Loopback backend, publicly safe HTTPS origin.** The install pins `DORMOUSE_BIND_HOST=127.0.0.1` and refuses to proceed without it - (`docs/specs/relay.md` → Configuration). Port 3100 lets the service - coexist with `PORT=3000 pnpm dev:relay`. Serve is the private default; Funnel is safe - to enable because the application controls, not network privacy, govern - admission (`docs/specs/security-remote.md` → "Network posture"). + (`docs/specs/relay.md` → Configuration). Port 3100 keeps the installed + service off the dev Relay's default (rationale). Serve is the private + default; Funnel is safe to enable because the application controls, not + network privacy, govern admission + (`docs/specs/security-remote.md` → "Network posture (self-hosted)"). - **`DORMOUSE_ORIGIN` is durable WebAuthn identity**, derived from the node's - MagicDNS name. An installation recording a different origin stops the - installer, because rewriting silently invalidates the registered passkey and - every enrolled Burrow. + MagicDNS name; an installation recording a different origin stops the + installer rather than rewriting it + (`docs/specs/security-remote.md` → "Network posture (self-hosted)"). - **The install belongs to one user account.** Every installer refuses to run privileged — root on macOS/Linux, elevated on Windows — because that account - owning `config/` and `state/` is the whole credential posture. **On unix the - property is mode *and* owner**. Both unix `manage verify` implementations - assert them on `config/`, `state/`, `run/`, `config/relay.env`, and an unspent - enrollment offer. Windows checks the owning SID and DACL, rejecting a missing - access-rule set (`docs/specs/security-remote.md` → "Credentials at rest"). + owning `config/` and `state/` is the whole credential posture. Which paths + `manage verify` asserts that on, and by what per-platform means, is + `docs/specs/security-remote.md` → "Credentials at rest". - **A failed update is a failure.** The candidate release is health-checked on an ephemeral port against a throwaway state dir *before* `current` moves; if the live service then fails to answer, `current` is restored to `previous` and @@ -716,15 +714,14 @@ reports which mode is live rather than asserting either. Each fails silently unless encoded in the scripts: -- **`pnpm deploy --prod --legacy` poisons the workspace.** (All three.) It - rewrites pnpm's workspace-state file to production mode. **Snapshot and - restore that file on every exit**, including failed installs. -- **`mv -f tmp link` follows a symlink to a directory.** (macOS, Linux.) Used - to swap `current`, it silently leaves the old release selected. **Use - `rename(2)` on the link path and assert that `current` advanced.** -- **`pnpm` resolves to a `.ps1` before its `.CMD`.** (Windows.) The shim cannot - be launched as a process, so **take the first `Application`-typed resolution**, - not `(Get-Command pnpm).Source`. +- **`pnpm deploy --prod --legacy` poisons the workspace.** (All three.) + **Snapshot and restore pnpm's workspace-state file on every exit**, including + failed installs (rationale). +- **`mv -f tmp link` follows a symlink to a directory.** (macOS, Linux.) **Use + `rename(2)` on the link path and assert that `current` advanced** (rationale). +- **`pnpm` resolves to a `.ps1` before its `.CMD`.** (Windows.) **Take the + first `Application`-typed resolution**, not `(Get-Command pnpm).Source` + (rationale). - **Redirecting a native command's stderr inline sets `$?` to false.** (Windows, PowerShell 5.1.) **Route control-flow commands through `Invoke-Native`**; only the candidate probe and `run-relay.ps1` append @@ -746,14 +743,11 @@ Each fails silently unless encoded in the scripts: reports the install as unserved and points at `manage serve`. Test mode skips this unstable CLI probe. - **`systemctl --user` needs a real login session, not just a shell.** (Linux.) - Under `su`, or wherever no user manager runs, its `DBUS_SESSION_BUS_ADDRESS` - failure does not say what to do, so preflight checks - `systemctl --user show-environment` and names the fix — log in properly, or - `machinectl shell $USER@`. -- **`StandardOutput=append:` is systemd 240 or newer.** (Linux.) Older systemd - truncates the log on every restart, so `manage logs` would show only the - current run. Parse `systemctl --version` and refuse below 240 rather than - installing a unit whose logging silently lies. + Preflight checks `systemctl --user show-environment` and names the fix — log + in properly, or `machinectl shell $USER@` (rationale). +- **`StandardOutput=append:` is systemd 240 or newer.** (Linux.) **Parse + `systemctl --version` and refuse below 240** rather than installing a unit + whose logging silently lies (rationale). ### Operator surface and test hooks diff --git a/SELF_HOST.rationale.md b/SELF_HOST.rationale.md new file mode 100644 index 000000000..dfa8c4ee0 --- /dev/null +++ b/SELF_HOST.rationale.md @@ -0,0 +1,105 @@ +# Run the Dormouse Relay behind Tailscale — rationale + +> Informative companion to [SELF_HOST.md](SELF_HOST.md): evidence keyed by that +> file's headings. Nothing here is normative. + +## Prerequisites + +Why the Linux operator grant is the only step that needs root: `tailscaled` +exposes its local API over a root-owned socket, so an unprivileged +`tailscale serve` is refused outright. `--operator=$USER` widens that socket to +one account, which is a one-time administrative act rather than something an +installer should take on itself. + +## What the installer does + +Why the logs sit outside the install root on macOS and Linux: each platform has +a conventional log location a user already knows to look in +(`~/Library/Logs/...`, `$XDG_STATE_HOME/dormouse-relay`), and `manage logs` +would otherwise train people to look somewhere the OS's own tooling ignores. +Windows has no such convention for a per-user service, so `logs` stays inside +the root — which is why `purge` has to name the log directory separately on the +other two. + +Why `$XDG_STATE_HOME` rather than the install root on Linux: the install root +is `$XDG_DATA_HOME` data that should survive and be backed up, while logs are +regenerable state. Backing up the data directory then does not drag a log +archive with it, which matters because checkpoint 6 asks the user to back that +directory up. + +## Checkpoint 1: preflight + +Why the dev-Relay note no longer names a port: `relay/scripts/dev.mjs` reads an +unset, blank, or `0` `PORT` as "any free port", so a developer running +`pnpm dev:relay` does not contend for 3100 at all unless they ask to. The older +text claimed `PORT=3000 pnpm dev:relay` pinned the dev Relay to 3000, which is +true of that exact invocation but not of the default, and read as though 3100 +existed to dodge 3000. + +## Checkpoint 3: verify + +Why the Windows kill block selects by path rather than by `Name='node.exe'`: a +developer machine routinely has several unrelated `node.exe` processes, and the +`Dormouse Relay` command-line substring is not on the supervisor's own +`powershell.exe`. Matching the install root against both `ExecutablePath` and +`CommandLine` is the same predicate `Get-DormouseProcess` uses inside the +installer, and the traps below say why image name is never enough. + +## Invariants + +Why 3100 rather than 3000: the number only has to be one a casual dev server is +unlikely to take, and 3000 is the most contended port on a developer's machine. +Nothing depends on the value — `config/relay.env` carries it and every consumer +reads it back — so this is a default, not a constraint. + +Why a failed restore must not clear `previous`: `rollback_release` re-reads +`current` before deciding, and its call sites use `|| true`, which disables +`errexit` for that command. Without the re-read a restore that never landed +would still strip the rollback pointer, leaving an install running the rejected +release with nothing to roll back to. + +## Mechanism map + +Both Windows deviations are mechanism constraining one file, so the reasoning +lives at the code: the comment above `Set-ReleasePointer` in +`deploy/local/install-windows.ps1` for why the pointer is a file swapped with +`rename(2)` semantics, and the header of the `run-relay.ps1` here-string in the +same file for why Task Scheduler's restart-on-failure is not KeepAlive. What +belongs here is only the consequence for the runbook: both rows of the +mechanism map read as arbitrary Windows trivia unless you know neither has an +unprivileged equivalent of the macOS mechanism, and `manage verify` reads the +supervision loop out of the wrapper rather than trusting the task settings +because the task setting is defence in depth rather than the mechanism. + +## Mechanical traps + +Why the pnpm workspace-state file has to be snapshotted: `pnpm deploy --prod +--legacy` rewrites it to production mode, so an install run from a developer's +checkout leaves that checkout unable to resolve dev dependencies until the next +full `pnpm install`. A failed install does the same damage as a successful one, +which is why the restore is on every exit rather than on the success path. + +Why `mv -f tmp link` cannot swap `current`: with `link` an existing symlink to a +directory, `mv -f` moves the source *into* that directory rather than replacing +the link, and the old release stays selected with no error anywhere. + +Why `(Get-Command pnpm).Source` is the wrong resolution: PowerShell prefers the +`.ps1` shim, which is a script rather than an image and cannot be launched as a +process, so every invocation failed with a message about the shim rather than +about pnpm. + +Why the `DBUS_SESSION_BUS_ADDRESS` failure is caught in preflight: under `su`, +or anywhere no user manager runs for this uid, `systemctl --user` fails with a +message that names the missing variable and nothing a user can act on — after +the unit has already been written. + +Why systemd 240 is the floor: `StandardOutput=append:` arrived in 240. Older +versions accept the unit and truncate the log on every restart, so `manage +logs` would show only the current run and a crash loop would look like a single +clean start. + +Why the Windows cmdlet lint exists at all: no job in CI has a PowerShell, so +`deploy/local/install-windows.ps1` has no syntax gate but +`scripts/ps1-cmdlet-lint.mjs`. A repo-wide rename of the project's vocabulary +once rewrote all 147 `Write-Host` calls in that file to `Write-Burrow`, which +nothing caught until someone ran the installer. diff --git a/docs/specs/hosted.md b/docs/specs/hosted.md index b6f2b728c..d44143bc1 100644 --- a/docs/specs/hosted.md +++ b/docs/specs/hosted.md @@ -11,7 +11,7 @@ **Must pin locally packed core/auth packages through root pnpm overrides and commit archives, provenance, and lockfile together.** `vendor/build.json` records the source commit, dirty state, and archive hashes. No runtime import depends on a sibling checkout. The auth migrations remain owned by the package. -Source of truth: `auth` in `hosted/server/worker.ts`; `workerApp` in `hosted/server/worker-app.ts`; `migrations` in `hosted/server/migrations.ts`; `scripts/sync-pgstencil.mjs`. +Source of truth: `auth` in `hosted/server/worker.ts`; `workerApp` in `hosted/server/worker-app.ts`; `migrations` in `hosted/server/migrations.ts`; `scripts/sync-pgstencil.mjs`. Pinned by `hosted/server/tests/artifacts.test.ts`. ## Identity and login @@ -35,7 +35,7 @@ Source of truth: `hosted/server/providers.js`; `authPolicy` / `providerBindings` **Must check the account on return to the page and serialize submitted actions.** Authenticated data remains in memory; login tokens never enter local storage. Only public identity fields are rendered, without provider images or external assets. -**Must inherit Dormouse product theme tokens before mounting React.** The OS light/dark preference selects bundled Light Visual Studio or Kimbie Dark. Hosted uses a narrow single-column form, 44px controls, 16px inputs, and 13px body copy; its page heading is 18px. It loads no marketing styles, fonts, or analytics. +**Must inherit Dormouse product theme tokens before mounting React.** The OS light/dark preference selects bundled Light Visual Studio or Kimbie Dark. Its type scale and touch sizing are in `hosted/src/style.css`. It loads no marketing styles, fonts, or analytics. Source of truth: `App` in `hosted/src/App.tsx`; `restoreTheme` in `hosted/src/main.tsx`; `hosted/src/style.css`. @@ -43,7 +43,7 @@ Source of truth: `App` in `hosted/src/App.tsx`; `restoreTheme` in `hosted/src/ma **Must run local development with `dor ensure -- pnpm dev:hosted` inside Dormouse.** A single loopback origin serves Vite and Node auth, with a disposable development database. Host, Origin, and Fetch Metadata checks guard the local captured-email inbox; the production entry imports no inbox or test-control handler. -**Must verify the production Worker bundle and run the consumer's integration suite before release.** `pnpm test:hosted` needs Docker; root `pnpm test` skips it. The test entry alone injects the packed Better Auth deterministic module. Simulated callbacks do not certify provider registrations; production acceptance requires real browser login with each enabled provider and email delivery. +**Must verify the production Worker bundle and run the consumer's integration suite before release.** Root `pnpm test` runs the `hosted/scripts/*.test.mjs` deploy suites; `pnpm test:hosted`'s vitest half needs Docker and is skipped there. The test entry alone injects the packed Better Auth deterministic module. Simulated callbacks do not certify provider registrations; production acceptance requires real browser login with each enabled provider and email delivery. **Must keep production, test, and preview databases and credentials separate.** The development and preview entries are email-only. Production configuration and operator steps live in `hosted/README.md`. @@ -63,9 +63,9 @@ Source of truth: `touchesHosted` in `hosted/scripts/changed.mjs`; `.github/workf ## Production releases -**Must deploy only manually selected main revisions after Hosted tests/build and accepted clean package provenance.** Preflight checks archive hashes, uncached Hyperdrive, matching migration/runtime database identity with distinct roles, and required Worker secret names. Back up, encrypt, decrypt, and restore-test before applying migrations; upload only the encrypted archive. Production has no public candidate URL. +**Must deploy only manually selected main revisions after Hosted tests/build and accepted clean package provenance.** `verifyPackages` checks the archive hashes; preflight checks uncached Hyperdrive, matching migration/runtime database identity with distinct roles, and required Worker secret names. Back up, encrypt, decrypt, and restore-test before applying migrations; upload only the encrypted archive. Production has no public candidate URL. -**Must record an immutable annotated hosted/YYYY-MM-DD tag only after live verification.** Dates use America/Los_Angeles; later deployments use numeric `--r2`, `--r3` suffixes. Tags identify the deployed commit and verification run/attempt. Tag retries are idempotent; redeployments get new tags. Code rollback never reverses migrations. +**Must record an immutable annotated hosted/YYYY-MM-DD tag only after live verification.** Tags identify the deployed commit and verification run/attempt; retries are idempotent and redeployments get new tags. Dating and repeat-deployment suffixes: `recordDeployment`. Code rollback never reverses migrations. Source of truth: `.github/workflows/hosted-production.yml`; `verifyPackages` / `preflight` in `hosted/scripts/production.mjs`; `hosted/scripts/production-backup.mjs`; `recordDeployment` in `hosted/scripts/production-tag.mjs`. Pinned by `hosted/scripts/production.test.mjs` and `hosted/scripts/production-tag.test.mjs`. diff --git a/docs/specs/security-audit.md b/docs/specs/security-audit.md index 21d3b69c4..b97bfd080 100644 --- a/docs/specs/security-audit.md +++ b/docs/specs/security-audit.md @@ -8,14 +8,14 @@ `.github/workflows/security-audit.yaml` audits `docs/specs/security.md` and the specs it names: nightly at `04:21 UTC` (`schedule`), on `workflow_dispatch`, and on the release tag, dispatched by `.github/workflows/release.yml` whose `publish-vscode` job `needs:` it — so no release ships without a passing audit. Dispatched, not `uses:`-called — see `docs/specs/security-ci.md` -> "GitHub Actions Policies". - **Must execute every `FAIL IF` as a mechanical check** (`gh api`, grep, file read, or a script run) **and add a qualitative pass** for security holes the specs do not cover. -- **`FAIL IF` lines are grouped by the operation that answers them**: one bullet may assert several properties when a single API call, file read, or script run establishes all of them. **Every clause stays an independent check**, with its own PASS/FAIL and its own evidence; never satisfied in bulk. -- **On any `FAIL IF` violation or BLOCKER-severity finding the workflow opens (or updates) a `security-audit-failure` issue** carrying the report, and exits non-zero; a subsequent passing audit auto-closes it. +- **`FAIL IF` lines are grouped by the operation that answers them**, one bullet asserting several properties when a single call establishes all of them. **Every clause stays an independent check**, with its own PASS/FAIL and evidence; never satisfied in bulk. +- **On any `FAIL IF` violation or BLOCKER-severity finding the audit fails**; [Outcomes and reporting](#outcomes-and-reporting) says what that files. - **FAIL IF** `.github/workflows/security-audit.yaml` is missing or disabled, or any of the three separate things that make it a release gate is gone: the `gh workflow run` dispatch, the `gh run watch --exit-status` that turns a failed audit into a failed job, and `publish-vscode`'s `needs:` edge on that job (rationale). ## Domains -**Must fan the CI audit out to three subagents with disjoint scopes**; the orchestrator audits nothing itself, spawning them concurrently and merging what they return (rationale). +**Must fan the CI audit out to three subagents, each owning a disjoint share of the tree**; a domain reads outside its share — `application-security` runs the repo's own lints — but owns nothing there. The orchestrator audits nothing itself, spawning them concurrently and merging what they return (rationale). **Ownership is by file: every `docs/specs/security*.md` spec is in exactly one domain's scope**, declared as backticked repo paths in the bullet list under the `**Scope` line of its domain file in `.github/audit/`, and enforced by `scripts/spec-lint.mjs`. @@ -25,39 +25,38 @@ | `ci-and-secrets` | `docs/specs/security-ci.md`, `docs/specs/security-audit.md`, `docs/specs/security.md` | | `application-security` | `docs/specs/security-local.md`, `docs/specs/security-remote.md`, `docs/specs/security-hosted.md` | -**The separation is one of context, not of credential.** `AUDIT_PAT` is a step-level `env:` on the one job, so every subagent inherits it, and only the prompt tells `application-security` not to use it. **Known gap:** a prompt is not a control; a real separation needs a second job outside the `security-audit` environment, passing fragments as artifacts — worth doing, not done. Three contexts each *read* less; none *holds* less. +**The separation is one of context, not of credential.** `AUDIT_PAT` is a step-level `env:` on the one job, so every subagent inherits it, and only the prompt tells `application-security` not to use it. A prompt is not a control: three contexts each *read* less, none *holds* less. A known gap, staged as `## Future` -> Credential separation. **Must pin the mechanical domains to Sonnet and `application-security` to Opus in CI and locally** (rationale). -**Must keep shared CI/local prompts and their scope declarations in `.github/audit/`** (rationale). +**Must keep shared CI/local prompts and their scopes in `.github/audit/`** (rationale). **Must make the sequential local runner exit nonzero for a failed process, a missing or unfinished fragment, or any verdict other than exact `VERDICT: PASS`.** It uses the operator's `gh` authentication when no `AUDIT_PAT` is supplied; inaccessible local checks are inconclusive. -**The qualitative scopes are stated by subtraction, so adding a directory cannot orphan it** (rationale). +**The qualitative scopes are stated by subtraction, so adding a directory cannot orphan it** (rationale). `application-security` takes the remainder, worked out from `ls -A` rather than from a list. -- `ci-and-secrets` — `.github/` (including `.github/audit/`), `.config/`, `.claude/`, `.vscode/`, `scripts/`, and `website/public/`, plus any code anywhere that touches a secret (rationale). -- `supply-chain` — the dependency graph, the lockfile, and all of `website/` except `website/public/`, stated as a subtraction rather than as named subdirectories. `website/scripts/generate-deps.js` is in that set, so the generator behind the disclosed snapshot is audited whole, not just its `productDependencyFilters` array (rationale). -- `application-security` — **everything else**, worked out from `ls -A` rather than from a list, including `.impeccable/`. **Dotfile directories are named explicitly wherever they land**, here and in the prompt files. **The subtraction is recursive**: where another domain claims a subdirectory rather than a whole tree — as both do inside `website/` — the remainder of that tree belongs here. +- **Dotfile directories are named explicitly wherever they land**, in the prompt files as here. +- **The subtraction is recursive**: where a domain claims a subdirectory rather than a whole tree — as `supply-chain` does inside `website/` — the remainder of that tree belongs to `application-security`. - **FAIL IF** a `docs/specs/security*.md` spec is in no domain's scope, or in two, or a scope names a file that does not exist (rationale). - **FAIL IF** the audit stops fanning out to a dedicated `application-security` subagent scoped to the application specs in the Domains table, or that scope is merged back into a context that also carries the supply-chain or CI domains (rationale). -- **FAIL IF** `application-security` does not run on a stronger model than the mechanical domains, in **both** `.github/workflows/security-audit.yaml`'s `--agents` and `scripts/security-audit-local.sh` (rationale). +- **FAIL IF** `application-security` does not run on a stronger model than the mechanical domains, in **both** `.github/workflows/security-audit.yaml`'s `claude_args` — its `--model` sets the floor and its `--agents` raises that one domain — and `scripts/security-audit-local.sh` (rationale). - **FAIL IF** `.github/audit/` is missing a prompt file the workflow names, or `scripts/security-audit-local.sh` stops running the audit from those same files (rationale). - **FAIL IF** the union of the subagents' qualitative scopes does not cover every top-level path in the repository (rationale). -- **FAIL IF** `.github/audit/` or `.vscode/` is outside **every** consumer of `.github/workflows/workflow-audit.yaml`'s diff window — the commit list, `own_changes`, and both classifiers' refusals, whose half is *derived* from the single `WINDOW` array (`"${WINDOW[@]:1}"`). The security specs are deliberately *not* watched there (rationale). +- **FAIL IF** `.github/audit/` or `.vscode/` is outside **any** consumer of `.github/workflows/workflow-audit.yaml`'s diff window — the commit list, `own_changes`, and both classifiers' refusals, whose half is *derived* from the single `WINDOW` array (`"${WINDOW[@]:1}"`). Widening one consumer without the others is the failure. The security specs are deliberately *not* watched there (rationale). -Source of truth: `--agents` in `.github/workflows/security-audit.yaml`; `run_domain` in `scripts/security-audit-local.sh`. +Source of truth: the `**Scope` and `## Qualitative pass` sections of each domain prompt in `.github/audit/`; `claude_args` in `.github/workflows/security-audit.yaml`; `run_domain` in `scripts/security-audit-local.sh`. ## Orchestration -**Subagents launch in the background** — the Task tool returns an id, not a report — so an orchestrator that ends its turn to await a completion notification ends the whole run: one headless turn, nothing resumes it (rationale). +**Subagents launch in the background**, so an orchestrator that ends its turn to await a completion notification ends the whole run (rationale). - **The job's `timeout-minutes: 40` stays above the orchestrator's 32-minute wait deadline** (rationale). -- **`--allowed-tools` enforces none of this**: it only auto-approves and removes nothing. `Task`/`Agent` are allowed on purpose; only `Workflow` is denied. -- **Each subagent appends to its own fragment as it determines each result**, never holding findings for a write-up at the end — `audit-supply-chain.md`, `audit-ci-secrets.md`, `audit-application.md` — and the orchestrator concatenates them rather than retyping. Fragments upload with the transcript, so an orchestrator that dies mid-merge still ships what the domains found. -- **A fragment opens `VERDICT: INCONCLUSIVE` and closes with the literal ``**, its verdict rewritten once at the end. The sentinel is what every reader treats as finished; an unfinished fragment fails closed on its opening line (rationale). +- **`--allowed-tools` enforces none of this**, only auto-approving; `Task`/`Agent` are allowed on purpose and only `Workflow` is denied (rationale). +- **Each subagent appends to its own fragment as it determines each result**, never holding findings for a write-up at the end, and the orchestrator concatenates them rather than retyping; `AUDIT_FRAGMENTS` in `.github/workflows/security-audit.yaml` names the three. Fragments upload with the transcript, so an orchestrator that dies mid-merge still ships what the domains found. +- **A fragment opens `VERDICT: INCONCLUSIVE` and closes with the literal ``**, its verdict rewritten once at the end. **The sentinel, not existence, is what a reader treats as finished** (rationale). -- **FAIL IF** the orchestrator prompt stops requiring a non-turn-ending wait — a Bash `until` loop over the fragments' sentinels, **breaking on its own sub-cap under the ten-minute Bash cap** so every call ends by printing its answer, re-issued under a bounded 32-minute deadline **persisted to a file** (`$RUNNER_TEMP/audit-deadline`) rather than recomputed from `now` (rationale). +- **FAIL IF** the orchestrator prompt stops requiring a non-turn-ending wait — a Bash `until` loop over the fragments' sentinels, **breaking on its own sub-cap under the Bash cap the workflow sets** so every call ends by printing its answer, re-issued under a bounded 32-minute deadline **persisted to a file** (`$RUNNER_TEMP/audit-deadline`) rather than recomputed from `now`. That cap is `BASH_DEFAULT_TIMEOUT_MS` in `.github/workflows/security-audit.yaml`, set above the loop's 540-second break; the harness default is two minutes, under it (rationale). - **FAIL IF** the prompt permits ending the turn without `audit-report.md` (rationale). - **FAIL IF** a domain prompt lets findings be held for a write-up at the end, or the wait, the merge, or the verdict treats existence rather than the sentinel as a domain having reported (rationale). - **FAIL IF** the orchestrator can report `PASS` while a subagent left no report fragment — nor `FAIL`, unless some domain actually returned one: the prompt writes no status file when a fragment is missing and no domain failed, routing an audit that ran out of time to INCONCLUSIVE. Both exit non-zero and hold the release gate shut (rationale). @@ -72,13 +71,14 @@ Source of truth: `2. Wait without ending your turn`, `3. Merge`, and `4. The ver |---|---|---| | `PASS` | literally `PASS` | open failure issues auto-closed; exit zero | | `FAIL` | literally `FAIL` | issue filed or updated; exit non-zero | -| INCONCLUSIVE | missing, empty, or anything else | filed under the same label, titled `INCONCLUSIVE`, body reproducing the partial report and saying it is not a security finding; exit non-zero | +| INCONCLUSIVE | missing, empty, or anything else | filed under the same label, body reproducing the partial report and saying it is not a security finding; exit non-zero | +- **A title moves upward only.** A new issue is titled for its outcome; an append retitles an open issue for `FAIL` alone, so an inconclusive run cannot relabel one already carrying findings, and a PASS closes it rather than walking it back. - **Must write `audit-report.md` before `audit-status.txt`.** A partial report can support FAIL; PASS requires every domain's completed checks. - **Partial has three shapes**, each named in the INCONCLUSIVE issue: `UNVERIFIABLE` for a check reached but not determined; `_Incomplete …_` above a fragment cut off mid-report; `_No report …_` for a domain that never wrote one. The merged `## Summary` may likewise read `INCONCLUSIVE`, and **gives no coverage count for a cut-off domain** (rationale). - **With no `audit-report.md` the reporting step publishes each fragment verbatim under its own heading**, unmerged (rationale). - **Must return `VERDICT: INCONCLUSIVE` from a domain with any undetermined check unless it found a failure.** Only all-determined passing checks permit `VERDICT: PASS`; a domain's inconclusive verdict prevents a merged pass. -- **`STATUS` is assigned in exactly two places**: where the status file is parsed, and in the single escalation block, **which orders `FAIL` > `MISSING` > `PASS`** — a dissent can raise `MISSING` to `FAIL` and never the reverse, and a `FAIL` alongside missing or unreadable fragments still reports them. Both fragment guards run unconditionally and only record. +- **`STATUS` is assigned in exactly two places**: where the status file is parsed, and in the single escalation block, **which orders `FAIL` > `MISSING` > `PASS`** — a dissent can raise `MISSING` to `FAIL` and never the reverse, and a `FAIL` alongside missing or unreadable fragments still reports them. - **The report is truncated to 32,000 characters before posting**, head kept, by `scripts/clamp-issue-body.mjs` (self-tested by `scripts/clamp-issue-body-selftest.mjs`). The call is non-fatal; the `audit-transcript` artifact holds the report in full; `.github/workflows/workflow-audit.yaml` truncates its commit list the same way (rationale). - **Every run uploads the `audit-transcript` artifact, which is world-readable and not secret-masked** — 14-day retention, deep-linked from failure issues (rationale). @@ -92,17 +92,21 @@ Source of truth: `clampIssueBody` in `scripts/clamp-issue-body.mjs`; `Surface re ## Environment and `AUDIT_PAT` -The audit job declares `environment: security-audit`, **whose deployment-branch-policy admits only `main` and `v*` tags** — both admin-only by the rulesets in `docs/specs/security-ci.md` -> "Automated Maintainer (tend)", so a write-scoped bot cannot reach its secrets from a feature branch. +The audit job declares `environment: security-audit`, **whose deployment-branch-policy admits only `main` and `v*` tags** — both admin-only by the rulesets in `docs/specs/security-ci.md` -> "Automated Maintainer (tend)" (rationale). -- **Audit changes are iterated on `main` directly**: a `workflow_dispatch` from any other ref is rejected before any step runs. To experiment on a branch, widen the policy temporarily and revert after. +- **Audit changes are iterated on `main` directly**: a `workflow_dispatch` from any other ref is rejected before any step runs; experimenting on a branch means widening the policy temporarily. - **`AUDIT_PAT` is required.** A dedicated step verifies the secret is present before the audit step runs — after the checkout and install, not literally first — and refuses to continue otherwise (rationale). - **The PAT is fine-grained and read-only**: `Administration` + `Secrets` + `Environments`, scoped to `diffplug/dormouse` only, minted on an admin's account, stored env-scoped. - **No step may ever print `$AUDIT_PAT` or `$CLAUDE_CODE_OAUTH_TOKEN`.** The prompt passes the PAT only through an unexpanded `GH_TOKEN=` prefix, and `gh api` responses never carry secret values (rationale). -```bash -gh secret set AUDIT_PAT --env security-audit --repo diffplug/dormouse --body 'github_pat_…' -``` - - **FAIL IF** the step that verifies `AUDIT_PAT` is provisioned before the audit runs is removed or bypassed (rationale). Source of truth: `Verify AUDIT_PAT is provisioned` in `.github/workflows/security-audit.yaml`; `Never print a secret value` in `.github/audit/_preamble.md`. + +## Future + +### Credential separation + +A second job outside the `security-audit` environment, running the domains that +need no PAT and passing their fragments back as artifacts, would leave +`application-security` unable to hold `AUDIT_PAT` at all. diff --git a/docs/specs/security-audit.rationale.md b/docs/specs/security-audit.rationale.md index 62bd75350..4773547ee 100644 --- a/docs/specs/security-audit.rationale.md +++ b/docs/specs/security-audit.rationale.md @@ -83,3 +83,9 @@ A bot-pushed feature branch cannot reach the audit job at all — GitHub rejects Without the PAT the audit cannot read the administration endpoints behind ruleset bypass actors, repo-level secret listing, and environment policies, so the specs it enforces would be unenforceable in their key sections. Passing the PAT only as an unexpanded `GH_TOKEN=` prefix is a convention, not a control: the agent holds unrestricted Bash and audits code that touches secrets, so one `printenv` or one `set -x` would publish an admin-read PAT for the artifact's whole retention. + +Provisioning the secret, for whoever has to rotate it: + +```bash +gh secret set AUDIT_PAT --env security-audit --repo diffplug/dormouse --body 'github_pat_…' +``` diff --git a/docs/specs/security-ci.md b/docs/specs/security-ci.md index 220ac0684..b159b712a 100644 --- a/docs/specs/security-ci.md +++ b/docs/specs/security-ci.md @@ -11,7 +11,7 @@ **Release audit dispatch.** The `security-audit` job in `.github/workflows/release.yml` holds `actions: write` and **may use it solely to** dispatch `security-audit.yaml` on the release tag and watch that run, gating the VS Code publish on the result (rationale). Blast radius: that job's `GITHUB_TOKEN` may start or cancel workflow runs in this repo, but cannot reach env-scoped secrets, merge to `main`, or push tags, and `release.yml` runs only on admin-gated `v*` tags. -- **FAIL IF** `pull_request_target` appears in any `.github/workflows/**` file other than `tend-*.yaml`. +- **FAIL IF** `pull_request_target` appears as an `on:` trigger in any `.github/workflows/**` file other than `tend-*.yaml`. A mention in a comment is not a violation. - **FAIL IF** a non-agent-managed workflow has **effective** write permissions other than the explicitly scoped release provenance permissions `id-token: write` and `attestations: write`, or the `actions: write` granted to the `security-audit` job in `release.yml` (see "Release audit dispatch"). Effective as in the agent-managed bullet below. ## Automated Maintainer (tend) @@ -40,7 +40,7 @@ This repository runs the [tend](https://github.com/max-sixty/tend) agent harness **Org-level secrets.** An org secret shared with this repo is reachable by any workflow the bot can author, exactly like a repo-level one, and does not appear in this repo's own secret listing — `gh api repos/diffplug/dormouse/actions/organization-secrets` is the check. **None are visible here today** (rationale); **must re-evaluate and name any that becomes visible before accepting it**, and the `FAIL IF` below admits none. -**Upstream compromise.** Every generated workflow references tend's action as `max-sixty/tend/claude@` — a **tag**, not a commit SHA, and mutable by whoever owns that repository, so upstream can change what our workflows execute with no commit landing here and `workflow-audit.yaml` seeing a byte-identical file. **A real residual, accepted** (rationale). **The version pin bounds *deliberate* upgrades, not a hostile upstream**; `uvx tend@latest` runs only at install and during nightly regen, so a compromise of that path affects the next re-run, not the in-flight workflows. **A second publisher now sits in the same position**: `tend-mention`'s `verify` and `tend-notifications`' `check` run `astral-sh/setup-uv@`, whose `uv` then interprets a `run:` step holding `TEND_BOT_TOKEN` — a broader trust than tend's, **accepted on the same generated-file grounds** (rationale). +**Upstream compromise.** Every generated workflow references tend's action as `max-sixty/tend/claude@` — a **tag**, not a commit SHA, and mutable by whoever owns that repository, so upstream can change what our workflows execute with no commit landing here and `workflow-audit.yaml` seeing a byte-identical file. **A real residual, accepted** (rationale). **The version pin bounds *deliberate* upgrades, not a hostile upstream**; `uvx tend@latest` runs only at install and during nightly regen, so a compromise of that path affects the next re-run, not the in-flight workflows. **A second publisher now sits in the same position**: `tend-mention`'s and `tend-notifications`' jobs run `astral-sh/setup-uv@`, whose `uv` then interprets a `run:` step holding `TEND_BOT_TOKEN` — a broader trust than tend's, **accepted on the same generated-file grounds** (rationale). **Audit visibility.** `.github/workflows/workflow-audit.yaml` walks nightly every commit touching `.github/workflows/`, `.config/tend.yaml`, `.github/audit/`, or `.vscode/` since its previous successful run — **across all branches, not just `main`**, so a workflow pushed to a feature branch is seen even though it never opens a PR. **This enumeration and the job's `WINDOW` must name the same paths** (rationale). It reports the *unexplained*, classifying out two routine sources on independently checked provenance and content: @@ -115,5 +115,5 @@ The extension is published by GitHub Actions, and the publishing secrets `VSCE_P **Known gap.** The documented remedy for `APPLE_SIGN_PASS` is `notarytool store-credentials` plus `--keychain-profile`, moving the exposure to one short call instead of every submission. Not yet done — it changes the release runbook and cannot be exercised without live Apple credentials. -- **FAIL IF** `scripts/sign-and-deploy.sh` stops doing any of three things: verifying GitHub artifact attestations, verifying artifact SHA-256 manifests, or using PIV-backed Windows signing. +- **FAIL IF** `scripts/sign-and-deploy.sh` stops doing any of three things: verifying GitHub artifact attestations, verifying artifact SHA-256 manifests, or using PIV-backed Windows signing. Pinned by `scripts/sign-and-deploy.test.mjs`. - **FAIL IF** `TAURI_SIGNING_PRIVATE_KEY` is passed on a command line anywhere in `scripts/sign-and-deploy.sh` rather than through the environment, or `EV_SIGN_PIN` is passed literally to `jsign --storepass` instead of by environment-variable reference. diff --git a/docs/specs/security-ci.rationale.md b/docs/specs/security-ci.rationale.md index 8a3fc5db3..834634cd3 100644 --- a/docs/specs/security-ci.rationale.md +++ b/docs/specs/security-ci.rationale.md @@ -10,7 +10,7 @@ **Why instruction files are a class of their own.** They are not read as data the way a diff is; Claude Code loads them as authoritative guidance, which is what makes a fork PR's copy of them a different class of input from the fork's code. -**The `0.1.18` gap, reported from this audit and now fixed.** At the previously pinned `0.1.18` the revert list was a flat, root-relative `SENSITIVE` array naming `CLAUDE.md` but no `AGENTS.md` at all — and this repo keeps its instructions in `AGENTS.md` with `CLAUDE.md` as a one-line `@AGENTS.md` pointer, so the control reverted a pointer and left the content it pointed at attacker-controlled. The fix ([max-sixty/tend#1005](https://github.com/max-sixty/tend/pull/1005), merged 2026-08-22, released in `0.1.19` on 2026-08-26) replaces that list with pathspec globs — `':(glob)**/AGENTS.md'`, `':(glob)**/CLAUDE.md'`, `':(glob)**/.claude/**'` — which `restore-sensitive-config.sh` passes to `pin_to_base`, covering every depth rather than a hand-enumerated set of root paths. The checked-in workflows use `0.2.7` as inspected in September 2026; `0.1.19` remains the minimum security floor. +**The `0.1.18` gap, reported from this audit and now fixed.** At the previously pinned `0.1.18` the revert list was a flat, root-relative `SENSITIVE` array naming `CLAUDE.md` but no `AGENTS.md` at all — and this repo keeps its instructions in `AGENTS.md` with `CLAUDE.md` as a one-line `@AGENTS.md` pointer, so the control reverted a pointer and left the content it pointed at attacker-controlled. The fix ([max-sixty/tend#1005](https://github.com/max-sixty/tend/pull/1005), merged 2026-08-22, released in `0.1.19` on 2026-08-26) replaces that list with pathspec globs — `':(glob)**/AGENTS.md'`, `':(glob)**/CLAUDE.md'`, `':(glob)**/.claude/**'` — which `restore-sensitive-config.sh` passes to `pin_to_base`, covering every depth rather than a hand-enumerated set of root paths. The checked-in workflows use `0.2.11` as inspected in September 2026; `0.1.19` remains the minimum security floor. **The local remedy if it ever regresses.** The nightly regen overwrites the *workflow*, not this repository's instruction files, so moving the instruction body into `CLAUDE.md` and dropping the pointer would close it with no upstream dependency, at the cost of the filename convention other agent harnesses read. diff --git a/docs/specs/security-hosted.md b/docs/specs/security-hosted.md index 47b476301..375d7edc2 100644 --- a/docs/specs/security-hosted.md +++ b/docs/specs/security-hosted.md @@ -2,7 +2,7 @@ > See `docs/specs/glossary.md` for Burrow, Client, and Relay vocabulary. > Owns the account application's security checks. Defers identity behavior to `docs/specs/hosted.md` and terminal access to `docs/specs/remote-security-model.md`. -> Read `docs/specs/security.md` first. Provisioning and real-provider acceptance remain pending. +> Read `docs/specs/security.md` first; provisioning and real-provider acceptance are pending. ## Origin boundary @@ -29,8 +29,10 @@ Pinned by `hosted/server/tests/workers.test.ts` and `hosted/server/tests/policy. - **FAIL IF** preview mail or OAuth calls reach external providers, preview configuration copies production routes/bindings, or a preview exposes deterministic time controls; inspect `hosted/server/preview-worker.ts`, `hosted/scripts/preview.mjs`, and `hosted/server/tests/workers.test.ts`. -Production activation must verify uncached Hyperdrive, separate credentials, and excluded marketing injection using `hosted/README.md`; checked-in placeholders do not prove those external controls. +Pinned by `hosted/server/tests/artifacts.test.ts`, `hosted/server/tests/workers.test.ts`, `hosted/server/tests/policy.test.ts`. + +Production activation must verify uncached Hyperdrive, separate credentials, and excluded marketing injection (`hosted/README.md`); checked-in placeholders prove none of them. ## Future -Public hosted voice and Relay require their own abuse, authorization, data-disclosure, and recovery checks before activation; `docs/specs/hosted.md` owns the staged work. +Public hosted voice and Relay need their own abuse, authorization, data-disclosure, and recovery checks first; `docs/specs/hosted.md` owns the staged work. diff --git a/docs/specs/security-local.md b/docs/specs/security-local.md index 57eac27c9..1b42d5d53 100644 --- a/docs/specs/security-local.md +++ b/docs/specs/security-local.md @@ -117,7 +117,7 @@ ancestor chain the webview supplies with each proxy URL request.** `'self'` allo same-grant nesting; any foreign ancestor fails. No request header identifies the embedder, and the browser checks the whole chain (rationale). -- **FAIL IF** any loopback HTTP or WebSocket listener grants an unrecognized caller a privilege it could not obtain by reaching the upstream directly. Refusing the request is one way; the iframe proxy's *admits all, vouches for none, names its embedder* is another, and is not a violation (rationale). `scripts/loopback-lint.mjs` (`pnpm test`) scans all tracked JavaScript and TypeScript and prints every bind it recognizes, with tests and its own fixtures separate from non-test listeners. A new non-test listener without a guard reference fails the build, but the lint cannot tell whether every request calls that guard. **Adding a server dependency means adding its bind spelling to `BIND_FORMS`**, each form pinned by `scripts/loopback-lint-selftest.mjs`; a host built at runtime is invisible to a regex in any spelling. Search the same files for `createServer`, `.listen(`, `serve(` and `WebSocket` to cover that ceiling. The Relay is separate: no foreign browser origin may drive its API, whatever interface it binds (`docs/specs/security-remote.md` -> "Cross-origin access"). A Unix-domain socket or named pipe is out of scope — no browser can reach one — which is why the `dor` control channel is bounded by socket permissions instead. +- **FAIL IF** any loopback HTTP or WebSocket listener grants an unrecognized caller a privilege it could not obtain by reaching the upstream directly. Refusing the request is one way; the iframe proxy's *admits all, vouches for none, names its embedder* is another, and is not a violation (rationale). `scripts/loopback-lint.mjs` (`pnpm test`) scans all tracked JavaScript and TypeScript and prints every bind it recognizes, with tests and its own fixtures separate from non-test listeners. A new non-test listener without a guard reference fails the build, but the lint cannot tell whether every request calls that guard. **Adding a server dependency means adding its bind spelling to `BIND_FORMS`**, each form pinned by `scripts/loopback-lint-selftest.mjs`. The Relay is separate: no foreign browser origin may drive its API, whatever interface it binds (`docs/specs/security-remote.md` -> "Cross-origin access"). A Unix-domain socket or named pipe is out of scope — no browser can reach one — which is why the `dor` control channel is bounded by socket permissions instead. - **FAIL IF** the iframe proxy rewrites `Origin` to the upstream's own origin for a caller whose inbound `Origin` is not the proxy's own — in `handleRequest` **or** `handleUpgrade`. A foreign `Origin` must be forwarded untouched rather than blocked, so the upstream sees the truth and applies its own policy (rationale). - **FAIL IF** the iframe proxy forwards `Cookie` upstream or `Set-Cookie` downstream on HTTP or WebSocket handshakes, including refused upgrades. Pinned by `lib/src/host/iframe-proxy.test.ts` (rationale). - **FAIL IF** the iframe proxy stops checking that `Host` names its own grant port, on either path. Its per-grant ephemeral port and one-fixed-upstream binding are real mitigations but neither is a secret, so the `Host` check is what makes DNS rebinding fail. @@ -198,8 +198,8 @@ lands at the umask — readable by another local account wherever `` is shared (rationale). No log call carries PTY bytes; the `dor` control socket path does. A gap, not an accepted risk. -- **FAIL IF** `write_file_atomically` in `standalone/src-tauri/src/lib.rs` stops restricting the directory and the file it writes to the owning user on **every** platform `restrict_to_owner` has an arm for — `0700`/`0600` on unix, and on Windows a DACL protected from inheritance carrying exactly one ACE for the current user, asserted by `restrict_to_owner_leaves_one_owner_only_ace` — or if any of its three callers — `write_session_to` (the session snapshot), the window-geometry sibling beside it, and `write_notepad_archive_to` (the notepad archive) — stops going through it. `session_write_tightens_directory_and_existing_temp_file` pins unix modes; `session_permission_failures_preserve_previous_snapshot_without_writing_bytes` pins both failure gates. The mode reaches the temp file *before* any bytes are written (rationale). -- **FAIL IF** the VS Code notepad archive key — `NOTEPAD_ARCHIVE_KEY` in `vscode-ext/src/notepad-archive-store.ts` — is passed to `context.globalState.setKeysForSync`, directly or as part of any list. It holds captured terminal excerpts, their CWDs, and their Surface titles, and Settings Sync would copy them to every machine the account signs into. Nothing in the extension calls that API today, so the rule is kept by that call not existing; a call added for anything else must exclude this key. +- **FAIL IF** `write_file_atomically` in `standalone/src-tauri/src/lib.rs` stops restricting the directory and the file it writes to the owning user on **every** platform `restrict_to_owner` has an arm for — `0700`/`0600` on unix, and on Windows a DACL protected from inheritance carrying exactly one ACE for the current user, asserted by `restrict_to_owner_leaves_one_owner_only_ace` — or if **any** of its callers stops going through it. Enumerate them from the file rather than from this line: every writer under the state root is one, the legacy-transcript scrub and `arrivals.json` included. `session_write_tightens_directory_and_existing_temp_file` pins unix modes; `session_permission_failures_preserve_previous_snapshot_without_writing_bytes` pins both failure gates. The mode reaches the temp file *before* any bytes are written (rationale). +- **FAIL IF** the VS Code notepad archive key — `NOTEPAD_ARCHIVE_KEY` in `vscode-ext/src/notepad-archive-store.ts` — is passed to `context.globalState.setKeysForSync`, directly or as part of any list. It holds captured terminal excerpts, their CWDs, and their Surface titles, and Settings Sync would copy them to every machine the account signs into. Pinned by `never opts any key into Settings Sync` in `vscode-ext/test/notepad-archive-store.test.ts`, which goes red on any `setKeysForSync(` call in `vscode-ext/src/`: a call added for another key is a deliberate edit to that test, and must still exclude this one. Source of truth: `SESSION_STATE_KEY` in `vscode-ext/src/session-state.ts`, `ensureToken` in `vscode-ext/src/peer-link.ts`, `default_log_path` in @@ -213,9 +213,11 @@ Source of truth: `context` in `standalone/sidecar/pty-core.js`; `attachRouter` i ## Dor Tool configuration -**Must keep repo-local named Tools inert until the user grants trust through Dormouse chrome.** The control socket exposes lookup and launch, never a trust-grant verb. Pending approval spawns neither its terminal nor a helper. Approval workflow belongs to `docs/specs/dor-tool.md` → Trust. +Approval workflow, declaration and resolution belong to `docs/specs/dor-tool.md` → Trust, Declaring tools. -**Must keep named-tool inputs as argv until the renderer quotes them for the target shell.** User configuration is the local user's authority; a project name cannot replace a user Tool during user-only lookup. Resolution belongs to `docs/specs/dor-tool.md` → Declaring tools. +**Must keep repo-local named Tools inert until the user grants trust through Dormouse chrome.** The control socket exposes lookup and launch, never a trust-grant verb. Pending approval spawns neither its terminal nor a helper. + +**Must keep named-tool inputs as argv until the renderer quotes them for the target shell.** User configuration is the local user's authority; a project name cannot replace a user Tool during user-only lookup. **Must reject C0 and DEL characters in Tool argv, substituted argv, and local-file targets before launch**, including controls exposed by canonicalizing symlinks. Shell quotes do not protect terminal editing keys. String `run` remains explicit shell code. Source of truth: `hasShellInputControls` in `dor/src/commands/shell-quote.ts`; `resolveToolInput` in `lib/src/host/tool-input.ts`; `useDorControl` in `lib/src/components/wall/use-dor-control.ts`. Tests: `lib/src/host/tool-input.test.ts`, `lib/src/components/Wall.test.tsx`. diff --git a/docs/specs/security-remote.md b/docs/specs/security-remote.md index b0544331f..65a00ccf8 100644 --- a/docs/specs/security-remote.md +++ b/docs/specs/security-remote.md @@ -32,7 +32,7 @@ no plaintext relay route, and no reader for any of the pre-cutover frames. | --- | --- | --- | | Relay | account state, routing metadata | **no new authorization and no plaintext**. On an established session, availability only — drop, delay, reorder, or refuse, never read and never inject — and the first invalid ciphertext destroys the session. Web Push holds **confidentiality**, not **freshness**: a kept envelope re-delivers as current, accepted residual (rationale). A session switched to the [direct path](#direct-path) leaves it the lifecycle levers alone — it can still end that session by dropping a socket, but sees, delays, and reorders none of its traffic | | Setup password | one endpoint, `/api/burrow/enroll`, and thence a `burrowToken` | it registers **no** passkey — `/api/setup/*` takes a Burrow-minted setup token and nothing else — so it reaches an owner passkey only via the next row. `/api/burrow/enroll` accepts one other credential, the installer's enrollment offer: owner-only *at rest*, the whole of what the file mode protects, checked by possession over HTTPS rather than local identity, so a leaked token redeems remotely — bounded single-use, 24-hour expiry, permanently disabled by the first Burrow enrollment. Still **no Burrow access** | -| `burrowToken` | the Burrow's own relay traffic and, transitively, **account takeover**: it mints setup tokens at `/api/burrow/setup-token`, the only thing that registers an owner passkey | bounded three ways — single-use and dead 5 minutes after minting; revoking the Burrow (deleting its row from `burrows.json`) stops minting immediately *and* kills already-minted tokens, re-checked at both setup gates; a signed-in phone retires an unused token at `/api/setup/retire`. Still **no Burrow access**: pairing runs Noise IK against an invitation keypair the Burrow never sent anywhere (rationale) | +| `burrowToken` | the Burrow's own relay traffic and, transitively, **account takeover**: it mints setup tokens at `/api/burrow/setup-token`, the only thing that registers an owner passkey | bounded three ways — single-use and dead 5 minutes after minting; revoking the Burrow (deleting its row from `burrows.json`) stops minting immediately *and* kills already-minted tokens, re-checked at both setup gates; a signed-in phone retires an unused token at `/api/setup/retire`. Still **no Burrow access** (rationale) | | Synced or stolen passkey | sign-in, and the ability to *ask* | the paired Client static is missing, so `BurrowAcl` answers `client-not-paired` | | Client static | use in place; encrypted fallback also permits private-byte extraction by compromised same-origin code | connecting still needs the paired passkey's fresh assertion, and it authorizes exactly one Burrow | @@ -113,7 +113,7 @@ per-Burrow browser storage follows `docs/specs/remote-security-model.md` -> - **FAIL IF** `burrow_state_dir` in `standalone/src-tauri/src/lib.rs` stops calling `restrict_to_owner` on the state directory **before** spawning the sidecar — on Windows those Node modes are no-ops and Node cannot set an ACL, so the guarantee is held one layer down. That call carries both legs: a newly written enrollment file *inherits* the owner-only entry, and one a prior version already left under the `%LOCALAPPDATA%` ACL — with a live `burrowToken` in it — has that entry *propagated* onto it, the half `restrict_to_owner_leaves_one_owner_only_ace` covers with its pre-existing `before.json`. - **FAIL IF** `relay/src/start.ts` stops obtaining the setup password from `SetupPasswordStore.loadOrCreate(generateSetupPassword)`, `generateSetupPassword` stops using `crypto.randomBytes(32)`, `readConfig` reads `DORMOUSE_SETUP_PASSWORD` or any other setup-password input, or `SetupPasswordStore` stops refusing a persisted or generated value outside 64 lowercase hexadecimal characters. Pinned by `relay/test/config.test.mjs` and `relay/test/setup-password-store.test.mjs`. - **FAIL IF** `createApp` accepts anything but 64 lowercase hexadecimal characters as the setup password injected by the entrypoint; pinned by `relay/test/app.test.mjs`. -- **FAIL IF** any installer stops making `config/`, `state/`, and `config/relay.env` reachable only by the installing user — the effective property `manage verify` tests: no principal other than that user may appear in the effective permissions. macOS and Linux achieve it with `0700`/`0600` under `umask 077`; Windows with a DACL protected from inheritance carrying exactly one ACE, which is how `Protect-Path` does it today but is not itself the invariant — a path that inherits that single ACE from an already-locked parent satisfies the property, and `Test-OwnerOnly` deliberately accepts it. The Windows and Linux installers create `relay.env` and lock it before writing its contents (rationale). +- **FAIL IF** any installer stops making `config/`, `state/`, and `config/relay.env` reachable only by the installing user — the effective property `manage verify` tests: no principal other than that user may appear in the effective permissions. macOS and Linux achieve it with `0700`/`0600` under `umask 077`; Windows with a single owner-only ACE, whether the path carries it directly or inherits it from an already-locked parent. The Windows and Linux installers create `relay.env` and lock it before writing its contents (rationale). - **FAIL IF** `manage verify` stops checking mode **and** owner on `config/`, `state/`, `run/`, `config/relay.env`, and an unspent enrollment offer on macOS or Linux, or Windows `Test-OwnerOnly` stops checking the owner SID alongside the DACL, or accepts an empty access-rule set. A NULL DACL grants everyone access. `scripts/installer-verify-test.mjs` exercises the unix checks; `scripts/deploy-lint.mjs` and its self-test pin all three platforms (rationale). - **FAIL IF** `manage verify` stops walking the files inside `state/` on Windows, where `relay/src/state.ts`'s `0o600` is a no-op and they are covered by what they inherit from the directory. An enumeration that fails fails verify, because that walk is the only thing holding the property there (rationale). - **FAIL IF** any installer stops preserving an existing `config/relay.env` byte-for-byte across an update. Each installer names the installer-owned keys a preserved file lacks and stops; nothing is rewritten or regenerated over it (rationale). @@ -156,7 +156,8 @@ a grant would widen the guessing surface and buy no compatibility (rationale). "Credentials at rest" deterministic**: every installer must still contain the control each `FAIL IF` names, so a control deleted from one of the three fails a build. It is textual and cannot tell whether a control is *correct* — the audit owns that — and on -Windows, which nothing in CI can execute, it is the only automated signal at all. +Windows, which nothing in CI can execute, it is the only automated signal about those +controls; `scripts/ps1-cmdlet-lint.mjs` reads the same file for cmdlet syntax alone. `scripts/deploy-lint-selftest.mjs` deletes each matched control in turn and requires the lint to fail (rationale). @@ -193,8 +194,8 @@ controls. - **FAIL IF** any installer stops refusing to rewrite a `DORMOUSE_ORIGIN` that no longer matches the node's DNS name. - **FAIL IF** any installer stops refusing to run with elevated privileges — `id -u` on macOS and Linux, the `Administrator` role check on Windows (rationale). - **FAIL IF** an installer or `manage` names `tailscale funnel` or `AllowFunnel` at all — invoking it, judging its state, or changing it all begin there, and public reachability must exercise the application controls rather than become a forbidden deployment state. Held by `scripts/deploy-lint.mjs` as its one `forbidden` rule (rationale). -- **FAIL IF** any decision taken on Tailscale CLI or listener output is reached by piping that output into `grep -q`, or into a `head -1` that exits first; every such search is over text captured first. The `head -1` half binds every site whose 141 can still reach an `if` or an assignment — an inline substitution always, and a helper the moment the failing assignment is its last command or a caller invokes it outside `$( )` (rationale). -- **FAIL IF** any decision about whether Serve maps `/` to us — the install-time conflict gate, `manage verify`, and the uninstall that turns Serve off — is not additionally scoped to the root line with the port right-bounded: `/api` on this port is not `/` on it, and `127.0.0.1:31000` contains `127.0.0.1:3100`. The post-mutation `SERVE_AFTER` assertion is the one deliberate exception, since it asserts our own `serve --bg` landed rather than auditing a foreign config (rationale). +- **FAIL IF** any decision taken on Tailscale CLI or listener output is reached by piping that output into `grep -q`, or into a `head -1` that exits first; every such search is over text captured first, in a helper as much as inline (rationale). +- **FAIL IF** any decision about whether Serve maps `/` to us — the install-time conflict gate, `manage verify`, and the uninstall that turns Serve off — is not additionally scoped to the root line with the port right-bounded. The post-mutation `SERVE_AFTER` assertion is the one deliberate exception (rationale). - **FAIL IF** `scripts/installer-verify-test.mjs` stops driving `has_off_loopback` and `serve_state` over inputs larger than the pipe buffer, or stops pinning `serve_proxies_root`'s root scoping and port bound. `scripts/deploy-lint.mjs` holds that helper's `<<<` pattern and counts its consumers; `serve_root_target` is held by neither on purpose (rationale). ### What crosses the boundary @@ -220,7 +221,7 @@ a second unchecked resolution. - **FAIL IF** `/api/push/send` stops taking the `burrowId` from the Burrow's own token, begins selecting recipients when `recipients` is absent or empty, stops clamping them at `MAX_PUSH_QUERY_DELIVERY_IDS`, or if any read endpoint begins reporting on a delivery id the caller did not present. Possession of the 256-bit `deliveryId` is the whole authorization for the Client-facing push routes, so the Relay must never *list* one to a session. - **FAIL IF** the send route reads, rewrites, or logs notification text, or forwards anything but the sealed envelope plus the token's own `burrowId`. The Relay holds no key for it (`docs/specs/remote-security-model.md` -> "Push sealing"), so a route that could read a payload is one that was handed plaintext. The envelope's three fields must be copied individually rather than spread, since a spread would let a sending Burrow override its own token's `burrowId`. - **FAIL IF** a push stops being sealed per recipient, to that ACL record's own Client static, under a fresh salt — the construction is `docs/specs/remote-security-model.md` -> "Push sealing", `sealPush` / `openPush` in `remote-lib-common/src/security/push-seal.ts`, proven by `remote-lib-common/test/push-seal.test.mjs`. A Noise `CipherState`, a shared group key, or a reused salt each break it. `BurrowRuntime.sealPushForClient` hands `lib/src/remote/burrow/push-delivery.ts` a seal *capability* and never the Burrow's private key, and the worker in `lib/src/remote/pocket-app/sw.ts` is the only thing that opens one. -- **FAIL IF** push text stops being bounded with the shared `boundedPushText` on the Burrow before sealing, or re-bounded with it in `lib/src/remote/pocket-app/sw.ts` before `showNotification`. The worker is the sanitization sink: a worker that renders what it decrypted without re-bounding it leaves the property with one enforcer instead of two (rationale). +- **FAIL IF** push text stops being bounded with the shared `boundedPushText` on the Burrow before sealing, or re-bounded with it in `lib/src/remote/pocket-app/sw.ts` before `showNotification`. The worker is the sanitization sink (rationale). - **FAIL IF** the relay routes a Burrow-originated frame from a socket that is not the Client's current Burrow binding, or begins decoding, remembering, or acting on an `e2e` ciphertext. `relay/src/relay.ts` must route the `e2e` envelope and nothing else: it holds no gate, no challenge memory, and no notion of an authorized session (rationale). A Relay-side type import from the protocol-v1 half of `remote-lib-common/src/remote/wire.ts` is the leading indicator and fails the same way. ### Direct path @@ -247,7 +248,9 @@ These are the two real gaps in the shipped model, and they are gaps rather than accepted risks — we intend to close them (rationale). **Revocation has no mechanism.** `BurrowAcl.revokeClient` / `revokePasskey` exist and -have no callers; no relay frame carries a revocation; there is no management UI. +have no production callers — only `remote-lib-common/test/acl.test.mjs` and +`security-guarantees.test.mjs` reach them; no relay frame carries a revocation; there is +no management UI. Revoking a lost phone means hand-editing JSON on the Burrow **and restarting it**: `BurrowService.#startBurrow` reads the store once and hands the `BurrowRuntime` a snapshot for its whole lifetime, so an edit alone changes nothing that is running. The diff --git a/docs/specs/security-supply-chain.md b/docs/specs/security-supply-chain.md index 97d87d46d..a5eb983d3 100644 --- a/docs/specs/security-supply-chain.md +++ b/docs/specs/security-supply-chain.md @@ -29,7 +29,7 @@ The roots are `productDependencyFilters` in `website/scripts/generate-deps.js`. **Must exclude workspaces that install no artifact:** - `canopy` — a Storybook-only rendering lab no shipped build imports. -- `website` — runs in a visitor's browser rather than being installed anywhere, which is what makes "puts on a user's machine" the operative test (rationale). +- `dormouse-website` — runs in a visitor's browser rather than being installed anywhere (rationale). - `dormouse-hosted` — runs on Workers and in the browser; no installed desktop or selfhost artifact imports it. **External binaries are outside this graph by construction** — the user's shell, and the `agent-browser` CLI `dor ab` forwards to (`npm i -g agent-browser`, a dependency of nothing here, resolved off `PATH`). **Dormouse instead ships nothing that pulls them in silently** (rationale). @@ -38,14 +38,14 @@ The roots are `productDependencyFilters` in `website/scripts/generate-deps.js`. **Must reject unclassified workspaces and exclusions reachable from a product root before generating disclosure.** Runtime and optional edges count; development edges do not. `website/scripts/dependency-workspaces.test.js` pins coverage. -**An unresolvable dependency throws unless an optional-edge rule covers it.** `node-datachannel` — the sidecar's second native addon, beside `node-pty` — publishes one prebuilt package per platform, and pnpm installs only the host's. +**An unresolvable dependency throws unless an optional-edge rule covers it.** `node-datachannel` publishes one prebuilt package per platform, and pnpm installs only the host's. - **Optional, declared by an external package: skipped.** The bundle copies `standalone/sidecar/node_modules`, so a prebuild the addon alone declares (android, musl) reaches nobody. - **Optional, declared by a product root: described from a sibling in the same `optionalDependencies` block at the same exact version string** — published in lockstep, so the disclosure is identical on every machine. No such sibling installed throws. -**Bundled themes are disclosed outside that lockfile walk.** The themes compiled into every build (`lib/src/lib/themes/bundled.json`) come from OpenVSX extensions, not npm, so `website/scripts/generate-deps.js` appends the checked-in `lib/src/lib/themes/bundled-extensions.json` to the npm table instead. The two come from one run of `lib/scripts/bundle-themes.mjs` but both are committed and can drift, which the CI gate below cannot see (rationale). `lib/src/lib/themes/bundled-extensions.test.ts` pins them, joining on the `extensionId` each disclosure record carries: a bundled theme whose extension has no record, or a record with no bundled theme left, fails. **The join is on the extension set only** — `bundled.json` carries no version or license, so nothing pins a hand-edit to those published fields. +**Bundled themes are disclosed outside that lockfile walk.** The themes compiled into every build (`lib/src/lib/themes/bundled.json`) come from OpenVSX extensions, not npm, so `website/scripts/generate-deps.js` appends the checked-in `lib/src/lib/themes/bundled-extensions.json` to the npm table instead. Both files are committed and can drift (rationale). `lib/src/lib/themes/bundled-extensions.test.ts` pins them, joining on the `extensionId` each disclosure record carries: a bundled theme whose extension has no record, or a record with no bundled theme left, fails. **The join is on the extension set only** — `bundled.json` carries no version or license, so nothing pins a hand-edit to those published fields. -- **FAIL IF** `node website/scripts/generate-deps.js` changes `website/src/data/dependencies-npm.json`, `website/src/data/dependencies-cargo.json`, or `website/src/data/dependencies-runtime.json` when run against a clean working tree after `pnpm install --frozen-lockfile`. The install is a precondition: the generator walks real `node_modules` directories and throws rather than under-reporting if they are absent. +- **FAIL IF** `node website/scripts/generate-deps.js` changes `website/src/data/dependencies-npm.json`, `website/src/data/dependencies-cargo.json`, or `website/src/data/dependencies-runtime.json` when run against a clean working tree after `pnpm install --frozen-lockfile` (rationale). - **FAIL IF** `.github/workflows/ci.yml` stops running that generator under that same install precondition, or stops failing on a diff (rationale). - **FAIL IF** the disclosure omits a shipped workspace's graph or excludes a shipped package. Derive shipping routes from `pnpm-workspace.yaml` and the builds, not the enumeration above; the generator enforces classification, but cannot establish whether an exclusion is justified (rationale). @@ -58,9 +58,9 @@ Source of truth: `productDependencyFilters` / `excludedWorkspacePackages` / `opt - **Its version is pinned exactly in the root `package.json` under `devEngines.runtime.version`**, and the build is the authority. - **The supply-chain page reads the same pin**, so the disclosed version provably equals the runtime users receive (rationale). - **The pin is deliberate and manual** — no automated ecosystem tracks it; workflows that do not bundle the runtime may track the same pinned major. -- Locally, pnpm honours `devEngines` (`onFail: "download"`) so scripts run under the pinned Node; CI drives `actions/setup-node` from the same field, and `node-version-file: package.json` resolves by precedence: `volta.node`, then `devEngines.runtime`, then `engines.node`. +- Locally, pnpm honours `devEngines` (`onFail: "download"`) so scripts run under the pinned Node; CI drives `actions/setup-node` from the same field through `node-version-file: package.json` (rationale). -**Must check the Windows runtime version before changing its PE Subsystem field from console (3) to GUI (2).** Only that two-byte field is patched; GUI Node preserves piped sidecar stdio but cannot serve the CLI's inherited console handles (`docs/specs/standalone.md -> "Windows node subsystem"`; rationale). +**Must check the Windows runtime version before changing its PE Subsystem field from console (3) to GUI (2).** Only that two-byte field is patched (`docs/specs/standalone.md -> "Windows node subsystem"`; rationale). - **FAIL IF** the root `package.json` is missing `devEngines.runtime.version`, or its value is not an exact `MAJOR.MINOR.PATCH` Node.js version — a bare major such as `24` is not acceptable. - **FAIL IF** `standalone/src-tauri/build.rs` no longer runs `--version` on the binary it is about to bundle and fails the build unless it matches `package.json`'s `devEngines.runtime.version`, or if the check is skipped for any configuration the release matrix builds. One deliberate skip is permitted: `verify_node_version` cannot execute a foreign-arch binary, so it warns and returns when `host != target` — acceptable only while every entry in `release.yml`'s standalone matrix is host-native; a cross-compiled entry ships an unverified runtime and fails this check. diff --git a/docs/specs/security.md b/docs/specs/security.md index f28f4629f..a880ed6a0 100644 --- a/docs/specs/security.md +++ b/docs/specs/security.md @@ -37,7 +37,7 @@ last column means nothing cheaper does. | **A page in a browser pane cannot forge a host message.** In VS Code every host message carries a per-boot token it cannot read, and the standalone adapters have no inbox for it to post to. | [Browser panes](./security-local.md#browser-panes) | `lib/src/lib/platform/vscode-adapter.test.ts` | | **Only your own account can drive your terminals through `dor`.** The socket sits in a directory only you can open, and its token never crosses the wire. | [The dor control socket](./security-local.md#the-dor-control-socket) | `standalone/sidecar/dor-control-server.test.js` | | **A loopback listener grants a stranger nothing it could not get from the upstream directly.** | [Loopback Listeners](./security-local.md#loopback-listeners) | `scripts/loopback-lint.mjs` | -| **Current persistence writers never save terminal scrollback.** Standalone snapshots are owner-only; VS Code controls access to its own storage. Older snapshots may contain transcripts. Explicitly captured notepad excerpts, titles, and working directories are archived on Surface closure. | [Persisted state](./security-local.md#persisted-state) | audit | +| **Current persistence writers never save terminal scrollback.** Standalone snapshots are owner-only; VS Code controls access to its own storage. Older snapshots may contain transcripts. Explicitly captured notepad excerpts, titles, and working directories are archived on Surface closure. | [Persisted state](./security-local.md#persisted-state) | `cargo test` in `standalone/src-tauri` (the owner-only half); audit | | **Nothing but a human at the laptop can authorize a phone.** The only path into a Burrow's ACL is typing, on that Burrow, the two digits the phone shows, and the Burrow makes every access decision. | [Pairing](./remote-security-model.md#pairing), [Burrow Authorization](./remote-security-model.md#burrow-authorization) | `remote-lib-common/test/security-guarantees.test.mjs` | | **The Relay cannot read ceremony or terminal content or grant terminal access.** One end-to-end channel per ceremony carries content under keys the Relay never holds; account data and routing metadata remain visible. | [Trust Model](./remote-security-model.md#trust-model), [Residual metadata](./remote-security-model.md#residual-metadata) | `scripts/e2e-lint.mjs` | | **Push notifications are opt-in, and a push is sealed to the one phone that receives it.** | [Push sealing](./remote-security-model.md#push-sealing) | `remote-lib-common/test/push-seal.test.mjs` | @@ -53,7 +53,7 @@ last column means nothing cheaper does. | **Merging to `main` and creating a tag are admin-only**, and every workflow this repository authors pins its actions by commit. | [GitHub Actions Policies](./security-ci.md#github-actions-policies) | audit | | **The bot maintainer cannot merge, tag, or read a release secret**, and its token never enters its own environment. | [Automated Maintainer (tend)](./security-ci.md#automated-maintainer-tend) | `.github/workflows/workflow-audit.yaml`, nightly | | **Publishing the extension takes a second human's approval.** | [VS Code Extension Releases](./security-ci.md#vs-code-extension-releases) | audit | -| **Desktop binaries are signed locally.** CI never holds production signing or updater keys, and the signing script verifies CI's attestations and hashes first. | [Desktop Releases](./security-ci.md#desktop-releases) | audit | +| **Desktop binaries are signed locally.** CI never holds production signing or updater keys, and the signing script verifies CI's attestations and hashes first. | [Desktop Releases](./security-ci.md#desktop-releases) | `scripts/sign-and-deploy.test.mjs` | ## What is not defended @@ -105,21 +105,19 @@ Gaps rather than accepted risks: we intend to close them. WebSocket cookie headers are stripped, but `document.cookie` remains shared; cookie-authenticated iframe pages are unsupported ([Loopback Listeners](./security-local.md#loopback-listeners)). -- **VS Code's peer-link token has no Windows ACL applied by Dormouse**, and - `recovery.json` is written at the umask - ([Persisted state](./security-local.md#persisted-state)). -- **The standalone log file is written at the umask**, readable by another - local account wherever the temp directory is shared, and records the `dor` - socket path; no terminal output reaches it - ([Persisted state](./security-local.md#persisted-state)). +- **Neither VS Code's peer-link token nor the `recovery.json` beside it carries + a Windows ACL applied by Dormouse.** Both are written owner-only by unix + mode, which Windows makes a no-op; standalone locks its state directory + instead ([Persisted state](./security-local.md#persisted-state)). +- **The standalone log file is written at the umask** and records the `dor` + socket path ([Persisted state](./security-local.md#persisted-state)). - **Revocation has no mechanism.** Revoking a lost phone is editing the Burrow's ACL file and restarting the Burrow ([Revocation and the audit trail](./security-remote.md#revocation-and-the-audit-trail)). - **There is no audit trail.** Nothing records connects, attaches, denials, or writes ([same](./security-remote.md#revocation-and-the-audit-trail)). -- **The workflow audit's window has two evasions**: a backdated committer date, - and a branch pushed, run, and deleted before the nightly fetch - ([Automated Maintainer](./security-ci.md#automated-maintainer-tend)). +- **The workflow audit's window has two evasions**, both in how the window is + computed ([Automated Maintainer](./security-ci.md#automated-maintainer-tend)). - **The audit's three subagents share one credential.** Their contexts are separate; `AUDIT_PAT` is not ([Domains](./security-audit.md#domains)). - **The notarization password sits on a command line for up to half an hour** diff --git a/hosted/src/style.css b/hosted/src/style.css index 634039daa..de8d18540 100644 --- a/hosted/src/style.css +++ b/hosted/src/style.css @@ -1,3 +1,9 @@ +/* + * The Hosted type scale (docs/specs/hosted.md -> Interface owns the rule that + * it is a narrow single-column form on product theme tokens): 13px body copy, + * an 18px page heading, 16px inputs — below that iOS Safari zooms the form on + * focus — and a 44px minimum control height, the platform touch target. + */ :root { font-synthesis: none; } diff --git a/scripts/spec-lint-selftest.mjs b/scripts/spec-lint-selftest.mjs index 5714ce159..ef4251feb 100644 --- a/scripts/spec-lint-selftest.mjs +++ b/scripts/spec-lint-selftest.mjs @@ -11,14 +11,15 @@ * The spec the cases plant into is chosen at run time: one with a rationale * file, no `## Future` (a planted heading must not land after the fold), and * the most room under its word budget, so a case cannot go red for the budget - * instead of for its check. Check 15 (a large spec needs a rationale file) is - * a number, not a pattern, and cannot be planted without also tripping the - * structural checks, so it is not here. `scripts/lint-kit.mjs` owns the - * edit-and-restore. Valid-form cases also keep optional navigation maps - * compatible with targeted implementation pointers. + * instead of for its check. Check 17's case needs a spec that is *not* a + * security spec, which headroom alone cannot promise, so it picks the same way + * from the specs that qualify. Check 15 is a number rather than a pattern: its + * case removes a paired rationale file instead of planting text. + * `scripts/lint-kit.mjs` owns the edit-and-restore. Valid-form cases also keep + * optional navigation maps compatible with targeted implementation pointers. */ -import { existsSync, readdirSync, readFileSync, writeFileSync } from 'node:fs'; +import { existsSync, readdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { execFileSync, spawnSync } from 'node:child_process'; import assert from 'node:assert/strict'; import { join } from 'node:path'; @@ -26,14 +27,24 @@ import { makeSelftest, readRepoFile, repoRoot } from './lint-kit.mjs'; import { countWords } from './spec-md.mjs'; const budgets = JSON.parse(readRepoFile('scripts/spec-word-budgets.json')); -const SPEC = readdirSync(join(repoRoot, 'docs/specs')) +const BY_HEADROOM = readdirSync(join(repoRoot, 'docs/specs')) .filter((f) => f.endsWith('.md') && !f.endsWith('.rationale.md')) .map((f) => `docs/specs/${f}`) .filter((f) => existsSync(join(repoRoot, f.replace(/\.md$/, '.rationale.md')))) .filter((f) => !/^##\s+(?:\d+\.\s*)?Future\s*$/m.test(readRepoFile(f))) .map((f) => [f, budgets[f] - countWords(readRepoFile(f))]) - .sort((a, b) => b[1] - a[1])[0][0]; + .sort((a, b) => b[1] - a[1]) + .map(([f]) => f); +const SPEC = BY_HEADROOM[0]; +const NON_SECURITY_SPEC = BY_HEADROOM.find((f) => !/\/security[a-z-]*\.md$/.test(f)); +assert.ok(NON_SECURITY_SPEC, 'check 17 needs a non-security spec to plant into'); const RATIONALE = SPEC.replace(/\.md$/, '.rationale.md'); +// Check 15 pairs a root-level spec with a root-level rationale; removing that +// file is the only way to plant the defect it exists to catch. Asserted +// present first: deleting what is already gone would "hold" while proving +// nothing. +const ROOT_RATIONALE = 'SELF_HOST.rationale.md'; +assert.ok(existsSync(join(repoRoot, ROOT_RATIONALE)), `check 15 needs ${ROOT_RATIONALE} to remove`); const SOURCE = 'standalone/scripts/clean-dev-sidecar.mjs'; // a comment appended here disturbs nothing // Assembled at runtime so this file's own planted citations are invisible to // the citation check, which scans every tracked source file, this one included. @@ -52,6 +63,7 @@ const CASES = [ ['check 13: a citation of a spec that does not exist', SOURCE, `\n// ${spec('no-such-spec.md')} -> "Heading"\n`], ['check 13: an unbackticked citation of a missing spec, from a spec', SPEC, `\nSee ${spec('no-such-spec.md')} -> "Heading" for more.\n`], ['check 14: a rule stated in a rationale file', RATIONALE, '\n**Never plant rules here.**\n'], + ['check 17: an audited rule outside a security spec', NON_SECURITY_SPEC, '\n- **FAIL IF** this rule is audited by nobody.\n'], ]; const selftest = makeSelftest('spec-lint.mjs', '.spec-selftest.bak'); @@ -80,6 +92,12 @@ for (const [name, target, text] of CASES) { selftest.withAppended(target, text, `${name}\n planting this in ${target} stays green — spec-lint cannot see it`); } +selftest.withMutation( + ROOT_RATIONALE, + (path) => rmSync(path, { force: true }), + `check 15: a large root-level spec with no rationale file\n removing ${ROOT_RATIONALE} stays green — spec-lint cannot see it`, +); + // Check 16's failures need separate mutations so one ownership diagnostic // cannot hide another one regressing. const DOMAIN = '.github/audit/supply-chain.md'; diff --git a/scripts/spec-lint.mjs b/scripts/spec-lint.mjs index cff6d553d..92065adf4 100644 --- a/scripts/spec-lint.mjs +++ b/scripts/spec-lint.mjs @@ -60,13 +60,18 @@ * spec. * 15. A spec of RATIONALE_REQUIRED_WORDS or more has a rationale file; * without one every piece of evidence sits above the fold and the - * ratchet cannot see it. + * ratchet cannot see it. Root-level specs ride this too: the rationale + * sits beside its spec, so SELF_HOST.md pairs with SELF_HOST.rationale.md. * 16. Every security spec (docs/specs/security*.md) is claimed by exactly one * audit domain: the bullet list under the `**Scope` line of a domain * prompt in .github/audit/ names it, and names no file that does not * exist. docs/specs/security-audit.md -> "Domains" states the rule; a * spec claimed by nobody is unaudited, one claimed twice gets * contradictory verdicts. + * 17. A `**FAIL IF**` rule leads a line only in a docs/specs/security*.md + * spec. AGENTS.md -> "House form for rules" states it; check 16 proves + * each security spec has an auditor, so an audited rule written anywhere + * else is claimed by nobody and silently never run. * * scripts/spec-lint-selftest.mjs plants one defect per finding check and * requires this lint to go red. @@ -93,7 +98,7 @@ const SKIP_PATH_PREFIXES = [ ]; // Root files a spec may name without a directory; every other checkable path // starts with a top-level dir. -const ROOT_FILES = ['AGENTS.md', 'DESIGN.md', 'PRODUCT.md', 'SECURITY.md', 'SELF_HOST.md', 'package.json', 'pnpm-workspace.yaml']; +const ROOT_FILES = ['AGENTS.md', 'DESIGN.md', 'PRODUCT.md', 'SECURITY.md', 'SELF_HOST.md', 'SELF_HOST.rationale.md', 'package.json', 'pnpm-workspace.yaml']; /** * A backticked token check 4 verifies on disk: a listed root file, or a repo @@ -113,15 +118,26 @@ const specDirFiles = readdirSync(join(ROOT, SPECS_DIR)) // Keep repo-relative paths in POSIX form on every platform — they are // compared against forward-slash paths written in the markdown. .map((f) => `${SPECS_DIR}/${f}`); -const rationaleFiles = specDirFiles.filter((f) => f.endsWith('.rationale.md')); const specFiles = specDirFiles.filter((f) => !f.endsWith('.rationale.md')); // SELF_HOST.md is a root-level spec (the self-host deployment: runbook + -// installer contract); it rides checks 2-4 alongside the docs/specs files. +// installer contract); it rides checks 2-4 alongside the docs/specs files, and +// checks 11 and 15 through ROOT_SPECS below. // SECURITY.md is the GitHub security policy — a pointer at the security specs, // budgeted so it cannot regrow into the 17,000-word spec it once was; it rides // the link, path, and budget checks only. +// +// A rationale file sits beside its spec, so a root-level spec's lives at the +// root too. Listing the spec rather than globbing the root keeps check 15 able +// to report a missing rationale: the pair is declared, not discovered. +const ROOT_SPECS = ['SELF_HOST.md']; +const rationaleFiles = [ + ...specDirFiles.filter((f) => f.endsWith('.rationale.md')), + ...ROOT_SPECS.map((f) => f.replace(/\.md$/, '.rationale.md')).filter((f) => existsSync(join(ROOT, f))), +]; const allFiles = ['AGENTS.md', 'SECURITY.md', 'SELF_HOST.md', ...specFiles, ...rationaleFiles]; const foldCheckedFiles = ['SELF_HOST.md', ...specFiles]; +/** Specs that pair with a rationale file — checks 11 and 15. */ +const rationaleCheckedSpecs = [...ROOT_SPECS, ...specFiles]; const problems = []; /** Memoize a one-argument pure function; the lint never writes, so nothing goes stale. */ @@ -374,7 +390,7 @@ for (const rel of Object.keys(budgets)) { // `(rationale)`, `(rationale; …)`, `(…; rationale)` — on whichever line the // wrap put it. const MARKER_RE = /(?:^|[(;])\s*rationale\s*[;)]/; -for (const spec of specFiles) { +for (const spec of rationaleCheckedSpecs) { const rat = spec.replace(/\.md$/, '.rationale.md'); const keys = hasRationale.has(rat) ? rationaleKeys(rat) : null; const heads = headings(spec); @@ -512,7 +528,7 @@ for (const rat of rationaleFiles) { // --- Check 15: a large spec has a rationale file ----------------------------- const RATIONALE_REQUIRED_WORDS = 2500; // stated in AGENTS.md -> "What, not why" -for (const spec of specFiles) { +for (const spec of rationaleCheckedSpecs) { const rat = spec.replace(/\.md$/, '.rationale.md'); const words = wordsOf.get(spec); if (!hasRationale.has(rat) && words >= RATIONALE_REQUIRED_WORDS) { @@ -554,6 +570,23 @@ for (const [spec, by] of claimants) { else if (by.length > 1) problems.push(`${spec}: in the scope of ${by.join(' and ')} — one domain owns a spec, or their verdicts contradict`); } +// --- Check 17: an audited rule lives only in a security spec ----------------- +// Check 16 gives every security spec exactly one auditor; nothing gives one to +// any other file, so a `FAIL IF` written elsewhere is never executed. Matched +// only where the bold LEADS the line — an inline `` `FAIL IF` `` pointer at the +// specs that own them is the form this leaves alone. +const FAIL_IF_RE = /^\s*(?:[-*]\s+)?\*\*FAIL IF\b/; +for (const rel of allFiles) { + if (claimants.has(rel)) continue; + proseLines(rel).forEach((line, i) => { + if (!FAIL_IF_RE.test(line)) return; + problems.push( + `${rel}:${i + 1}: a "**FAIL IF**" rule outside docs/specs/security*.md — no audit domain ` + + 'claims this file, so nothing runs it (AGENTS.md -> "House form for rules")', + ); + }); +} + // ----------------------------------------------------------------------------- if (problems.length > 0) { console.error(`spec-lint: ${problems.length} problem(s)\n`); diff --git a/scripts/spec-word-budgets.json b/scripts/spec-word-budgets.json index 9b5bf9e16..ac02de818 100644 --- a/scripts/spec-word-budgets.json +++ b/scripts/spec-word-budgets.json @@ -1,7 +1,7 @@ { "AGENTS.md": 3400, "SECURITY.md": 200, - "SELF_HOST.md": 6200, + "SELF_HOST.md": 6050, "docs/specs/alert.md": 6850, "docs/specs/auto-update.md": 1150, "docs/specs/deploy.md": 1850, @@ -22,8 +22,8 @@ "docs/specs/security-ci.md": 2700, "docs/specs/security-hosted.md": 450, "docs/specs/security-local.md": 3150, - "docs/specs/security-remote.md": 5850, - "docs/specs/security-supply-chain.md": 1250, + "docs/specs/security-remote.md": 5750, + "docs/specs/security-supply-chain.md": 1150, "docs/specs/security.md": 1900, "docs/specs/shortcuts.md": 1150, "docs/specs/standalone.md": 11500, diff --git a/website/src/pages/SecurityDocs.tsx b/website/src/pages/SecurityDocs.tsx index 3c1ad274a..fa03d67b9 100644 --- a/website/src/pages/SecurityDocs.tsx +++ b/website/src/pages/SecurityDocs.tsx @@ -46,7 +46,7 @@ export default function SecurityDocs() { supply-chain disclosure - . The five audited checklists behind all three live beside the spec, in{" "} + . The audited checklists behind all three live beside the spec, in{" "} the specs directory on GitHub From 4d5cbb4cbad275cec37dbaa1ec3ab0e09f9ca398 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sun, 20 Sep 2026 01:12:56 -0700 Subject: [PATCH 2/5] fix(self-host): close two manage verify gaps, and stop duplicating two values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two things SELF_HOST.md's "Definition of done" states for all three platforms were checked on fewer than three, so `manage verify` reported healthy on an install that was not. - **A dangling `previous` pointer.** Only Linux failed it. macOS and Windows compared `previous` to `current` and stopped, so a pointer at a pruned release passed green and `manage rollback` then failed on a release that was not on disk. Both now fail it, as Linux does. - **A credential in the service definition.** Nothing inspected the plist, the unit, the scheduled-task XML, or the `run-relay` wrapper for one: a plist carrying `DORMOUSE_SETUP_PASSWORD` passed verify. All three now search the definition and the wrapper for every credential name the installer knows, exempting the enrollment offer's `_FILE` path, which they legitimately carry. Both are deploy-lint rules with per-platform patterns, so deleting the control from any one installer fails the build, and the self-test's mutation proves each is load-bearing (112 -> 118 checks). Also: - `manage verify`'s "Serve does not proxy" failure prints the `manage serve` command that re-applies the mapping, on all three. - The audit workflow parses the status file into `FILE_STATUS` and assigns `STATUS` from it once, deleting `STATUS_FILE_VERDICT` — a derived variable with one reader that existed only because `STATUS` was mutated in place. - The Hosted account screen's recent-login window was a `600_000` literal beside the packed adapter's `freshAge: 10 * 60`; a pgstencil bump would have desynced the UI gate from the server gate, visible as a "connect" button that always 302s to `/login?error=`. One `LOGIN_FRESH_AGE_MS` in `hosted/server/policy.ts` now, pinned against the adapter's own built options by `hosted/server/tests/policy.test.ts`. Co-Authored-By: Claude Fable 5.1 --- .github/workflows/security-audit.yaml | 23 ++++++++-------- SELF_HOST.md | 16 ++++++----- deploy/local/install-linux.sh | 15 +++++++++++ deploy/local/install-macos.sh | 17 ++++++++++++ deploy/local/install-windows.ps1 | 16 +++++++++++ hosted/server/policy.ts | 8 ++++++ hosted/server/tests/policy.test.ts | 15 ++++++++++- hosted/src/App.tsx | 4 ++- scripts/deploy-lint.mjs | 39 +++++++++++++++++++++++++++ scripts/spec-word-budgets.json | 2 +- 10 files changed, 133 insertions(+), 22 deletions(-) diff --git a/.github/workflows/security-audit.yaml b/.github/workflows/security-audit.yaml index feb587b4b..446b358e1 100644 --- a/.github/workflows/security-audit.yaml +++ b/.github/workflows/security-audit.yaml @@ -299,21 +299,20 @@ jobs: # `security-audit-failure`: an audit that produced no verdict # must not let the release gate in `release.yml` pass, and a # later PASS should auto-close it like any other failure. + + # What the *file* said, parsed once and never mutated. Every note in + # the issue body is written about a condition rather than about a + # branch, and "the audit never wrote a verdict" is one of those + # conditions — it cannot be recovered from `STATUS` afterwards, + # because the escalation below can raise that same MISSING to FAIL. if [ -s audit-status.txt ]; then - STATUS=$(cat audit-status.txt) + FILE_STATUS=$(cat audit-status.txt) fi - case "${STATUS:-}" in + case "${FILE_STATUS:-}" in PASS|FAIL) ;; - *) STATUS=MISSING ;; + *) FILE_STATUS=MISSING ;; esac - - # What the *file* said, kept separate from `STATUS`, which the guards - # below mutate. Every note in the issue body is written about a - # condition rather than about a branch, and "the audit never wrote a - # verdict" is one of those conditions — it cannot be recovered from - # `STATUS` afterwards, because a dissent can raise that same MISSING - # to FAIL. - if [ "$STATUS" = "MISSING" ]; then STATUS_FILE_VERDICT=none; else STATUS_FILE_VERDICT="$STATUS"; fi + STATUS="$FILE_STATUS" # A PASS is only as good as the evidence behind it. Each domain # writes its own fragment before returning a verdict, so a @@ -488,7 +487,7 @@ jobs: if [ -n "${INCONCLUSIVE_DOMAINS:-}" ]; then echo "- **A domain could not determine every check.** $INCONCLUSIVE_DOMAINS returned \`VERDICT: INCONCLUSIVE\`; read its \`UNVERIFIABLE\` checks. Those checks do not count as passing." >> "$NOTES" fi - if [ "$STATUS_FILE_VERDICT" = "none" ]; then + if [ "$FILE_STATUS" = "MISSING" ]; then echo "- **The audit wrote no verdict.** \`audit-status.txt\` was absent, empty, or not \`PASS\`/\`FAIL\`, so the run ended without deciding. Start at the report's own markers: \`UNVERIFIABLE\` is a check the agent could not determine, \`_Incomplete …_\` heads a domain that was cut off mid-report, and a domain section reading \`_No report …_\` is one that never reported — which is what an expired wait deadline looks like." >> "$NOTES" fi diff --git a/SELF_HOST.md b/SELF_HOST.md index 26cd0dbfc..c90a290f6 100644 --- a/SELF_HOST.md +++ b/SELF_HOST.md @@ -176,10 +176,9 @@ what keeps the task free of a stored password; Linux alone opts out, with - **The service is registered and running, declares the run-at-load and restart-on-exit of the [Mechanism map](#mechanism-map), and carries no - credential** — a definition it cannot read at all fails rather than passes. - **No `manage verify` inspects the definition for a credential today**: the - installers put none there, and nothing checks that they still do not. - Plus what only the live system shows: macOS, loaded in `gui/$UID` with a plist + credential** — a definition it cannot read at all fails rather than passes, + and `verify` searches it and the `run-relay` wrapper for every credential + name the installer knows. Plus what only the live system shows: macOS, loaded in `gui/$UID` with a plist that lints; Windows, task `Running`, no execution time limit, restarts on failure, unelevated, unstopped by battery or idle, `bin\run-relay.ps1` still carrying the supervision loop; Linux, unit known to the user manager, @@ -191,8 +190,9 @@ what keeps the task free of a stored password; Linux alone opts out, with - **Port 3100 is bound only to `127.0.0.1`**, and the plaintext port is unreachable on the laptop's Tailscale IP. - **`tailscale serve` proxies `/` to `127.0.0.1:3100` at the origin recorded in - `config/relay.env`.** Funnel may be on or off; verification does not treat - public HTTPS reachability as a defect. + `config/relay.env`.** A failure prints the `manage serve` command that + re-applies it. Funnel may be on or off; verification does not treat public + HTTPS reachability as a defect. - **`config/`, `state/`, `run/` and `config/relay.env` are readable only by the installing user**, by the per-platform means in the [Mechanism map](#mechanism-map) and [Invariants](#invariants). @@ -201,7 +201,9 @@ what keeps the task free of a stored password; Linux alone opts out, with - **The current release pointer resolves to a release with `RELEASE` metadata**, and neither the service definition nor the `run-relay` wrapper refers to the source checkout. The previous-release pointer is checked too: - absent on a first install warns, naming the same release as `current` fails. + absent on a first install warns, while naming the same release as `current`, + or a release that is no longer on disk, fails — `manage rollback` would + otherwise be offered a target that is not there. These cannot be proven from the laptop, and are the checkpoints below: the HTTPS origin answering from a second tailnet device and, when private HTTPS is intended, diff --git a/deploy/local/install-linux.sh b/deploy/local/install-linux.sh index ead550211..19da74748 100755 --- a/deploy/local/install-linux.sh +++ b/deploy/local/install-linux.sh @@ -1146,6 +1146,9 @@ cmd_verify() { pass "Serve proxies / to 127.0.0.1:$PORT" else fail "Serve does not proxy / to 127.0.0.1:$PORT" + # The remedy is one command and `verify` already knows it, so print it + # rather than leaving the reader to find `manage serve` in the runbook. + printf ' re-apply it with: "%s/bin/manage" serve\n' "$ROOT" printf '%s\n' "$serve_out" | sed 's/^/ /' fi if [ -n "$ORIGIN" ] && grep -q "${ORIGIN#https://}" <<<"$serve_out"; then @@ -1197,6 +1200,18 @@ cmd_verify() { warn "no previous release retained yet — rollback is unavailable until the next update" fi + # The service definition and the wrapper carry the enrollment offer's *path* + # and nothing else secret (docs/specs/security-remote.md -> "Credentials at + # rest"). A name, not a value: the installer supplies none of these, so one + # appearing means a hand-edit or a regression put a credential where any + # process that can read the definition can read it. + if grep -qE 'DORMOUSE_SETUP_PASSWORD|DORMOUSE_VAPID_PRIVATE_KEY|DORMOUSE_ENROLL_TOKEN[^_]' \ + "$UNIT_FILE" "$ROOT/bin/run-relay" 2>/dev/null; then + fail "the unit or wrapper names a credential — it must carry only paths" + else + pass "the service definition names no credential" + fi + # The release must not depend on the source checkout. local src src="$(release_field source_checkout || echo '')" diff --git a/deploy/local/install-macos.sh b/deploy/local/install-macos.sh index d568b039e..6c7897cae 100755 --- a/deploy/local/install-macos.sh +++ b/deploy/local/install-macos.sh @@ -917,6 +917,9 @@ cmd_verify() { pass "Serve proxies / to 127.0.0.1:$PORT" else fail "Serve does not proxy / to 127.0.0.1:$PORT" + # The remedy is one command and `verify` already knows it, so print it + # rather than leaving the reader to find `manage serve` in the runbook. + printf ' re-apply it with: "%s/bin/manage" serve\n' "$ROOT" printf '%s\n' "$serve_out" | sed 's/^/ /' fi if [ -n "$ORIGIN" ] && grep -q "${ORIGIN#https://}" <<<"$serve_out"; then @@ -955,12 +958,26 @@ cmd_verify() { if [ -L "$ROOT/previous" ] && [ "$(readlink "$ROOT/previous")" = "$(readlink "$ROOT/current" 2>/dev/null)" ]; then fail "previous names the same release as current — there is no rollback target" + elif [ -L "$ROOT/previous" ] && [ ! -d "$ROOT/previous" ]; then + fail "the previous symlink points at a release that no longer exists" elif [ -L "$ROOT/previous" ]; then pass "a previous release is retained for rollback" else warn "no previous release retained yet — rollback is unavailable until the next update" fi + # The service definition and the wrapper carry the enrollment offer's *path* + # and nothing else secret (docs/specs/security-remote.md -> "Credentials at + # rest"). A name, not a value: the installer supplies none of these, so one + # appearing means a hand-edit or a regression put a credential where any + # process that can read the definition can read it. + if grep -qE 'DORMOUSE_SETUP_PASSWORD|DORMOUSE_VAPID_PRIVATE_KEY|DORMOUSE_ENROLL_TOKEN[^_]' \ + "$PLIST" "$ROOT/bin/run-relay" 2>/dev/null; then + fail "the LaunchAgent or wrapper names a credential — it must carry only paths" + else + pass "the service definition names no credential" + fi + # The release must not depend on the source checkout. local src src="$(release_field source_checkout || echo '')" diff --git a/deploy/local/install-windows.ps1 b/deploy/local/install-windows.ps1 index 972e4fef6..8bf3c60b9 100644 --- a/deploy/local/install-windows.ps1 +++ b/deploy/local/install-windows.ps1 @@ -1604,6 +1604,9 @@ function Invoke-Verify { Pass "Serve proxies / to 127.0.0.1:$PORT" } else { Fail "Serve does not proxy / to 127.0.0.1:$PORT" + # The remedy is one command and `verify` already knows it, so print it + # rather than leaving the reader to find `manage serve` in the runbook. + Write-Host (' re-apply it with: "{0}\bin\manage.cmd" serve' -f $Root) foreach ($l in $serveText.Split("`n")) { if ($l.Trim()) { Write-Host " $($l.TrimEnd())" } } } if ($ORIGIN -and ($serveText -match [regex]::Escape($ORIGIN.Replace('https://', '')))) { @@ -1684,12 +1687,25 @@ function Invoke-Verify { $prev = Get-PreviousRelease if ($prev -and $prev -eq $cur) { Fail "previous names the same release as current -- there is no rollback target" + } elseif ($prev -and -not (Test-Path -LiteralPath (Join-Path $Root "releases\$prev") -PathType Container)) { + Fail "previous.txt points at a release that no longer exists" } elseif ($prev) { Pass "a previous release is retained for rollback" } else { Warn "no previous release retained yet -- rollback is unavailable until the next update" } + # The task definition and the wrapper carry the enrollment offer's *path* and + # nothing else secret (docs/specs/security-remote.md -> "Credentials at + # rest"). A name, not a value: the installer supplies none of these, so one + # appearing means a hand-edit or a regression put a credential where any + # process that can read the definition can read it. + if (("$taskXml" + "`n" + $wrapperText) -match 'DORMOUSE_SETUP_PASSWORD|DORMOUSE_VAPID_PRIVATE_KEY|DORMOUSE_ENROLL_TOKEN(?!_FILE)') { + Fail "the Scheduled Task or wrapper names a credential -- it must carry only paths" + } else { + Pass "the service definition names no credential" + } + # The release must not depend on the source checkout. $src = Get-ReleaseField 'source_checkout' if ($src) { diff --git a/hosted/server/policy.ts b/hosted/server/policy.ts index 2943ee8a2..ecb698a4a 100644 --- a/hosted/server/policy.ts +++ b/hosted/server/policy.ts @@ -16,6 +16,14 @@ export const authPolicy = { // plain-node deploy scripts import without this module's dependencies. export { providerIds, type ProviderId }; +// How long after signing in a login still counts as recent enough to connect a +// provider. The packed adapter enforces it as Better Auth's `session.freshAge` +// and the account screen gates its buttons on the same window, so the two would +// desync silently on a pgstencil bump: a UI that offers "connect" for a login +// the server has already stopped accepting 302s to /login?error=. One value, +// and `hosted/server/tests/policy.test.ts` pins it to what the adapter builds. +export const LOGIN_FRESH_AGE_MS = 10 * 60 * 1000; + // Only an explicit deployment allowlist enables a provider; stale secrets do not. export function providerBindings(env: Record) { const enabled = String(env.OAUTH_PROVIDERS ?? "") diff --git a/hosted/server/tests/policy.test.ts b/hosted/server/tests/policy.test.ts index e080bcbf7..ab7dec63b 100644 --- a/hosted/server/tests/policy.test.ts +++ b/hosted/server/tests/policy.test.ts @@ -1,5 +1,5 @@ import { test, expect } from "vitest"; -import { providerBindings } from "../policy"; +import { authPolicy, providerBindings, LOGIN_FRESH_AGE_MS } from "../policy"; import { allowedDevRequest } from "../dev-host-guard"; import type { IncomingMessage } from "node:http"; test("provider allowlist fails closed on typos and partial credentials", () => { @@ -27,3 +27,16 @@ test("local inbox is guarded against rebinding and cross-origin requests", () => check({ host: "127.0.0.1:5188", "sec-fetch-site": "cross-site" }), ).toBe(false); }); +// The account screen and the packed adapter gate on the same window; nothing +// else would notice a pgstencil bump moving one of them. +test("the recent-login window matches the adapter's own freshAge", async () => { + const { authOptions } = await import("@pgstencil/auth/better-auth"); + const built = authOptions({ + ...authPolicy, + database: {} as never, + origin: "https://hosted.dormouse.sh", + secret: "x".repeat(32), + email: { send: async () => {} } as never, + }); + expect(built.session?.freshAge).toBe(LOGIN_FRESH_AGE_MS / 1000); +}); diff --git a/hosted/src/App.tsx b/hosted/src/App.tsx index ec7474f7a..a428b1454 100644 --- a/hosted/src/App.tsx +++ b/hosted/src/App.tsx @@ -16,6 +16,7 @@ import { type Provider, type Session, } from "./api"; +import { LOGIN_FRESH_AGE_MS } from "../server/policy"; export function App() { const [session, setSession] = useState(null); @@ -129,7 +130,8 @@ export function App() { ); }); const fresh = - session && Date.now() - Date.parse(session.session.createdAt) < 600_000; + session && + Date.now() - Date.parse(session.session.createdAt) < LOGIN_FRESH_AGE_MS; return (
diff --git a/scripts/deploy-lint.mjs b/scripts/deploy-lint.mjs index 491fe26d3..52e3d0635 100644 --- a/scripts/deploy-lint.mjs +++ b/scripts/deploy-lint.mjs @@ -224,6 +224,45 @@ export const RULES = [ 'the per-file walk is Windows-only, for the reason stated in the macOS skip; Linux checks mode and owner on `state/` itself', }, }, + { + // SELF_HOST.md -> "Definition of done" states this for all three, and only + // Linux held it: macOS and Windows compared `previous` to `current` and + // stopped, so a pruned rollback target passed `verify` green and + // `manage rollback` then failed on a release that was not there. + // + // Condition and verdict as one span, per platform: the message alone could + // survive the test being deleted, and the test alone could survive the + // branch reporting a pass. + rule: 'Definition of done — manage verify fails a dangling previous-release pointer', + patterns: { + macOS: + /\[ -L "\$ROOT\/previous" \] && \[ ! -d "\$ROOT\/previous" \]; then\n\s*fail "the previous symlink points at a release that no longer exists"/, + Linux: + /\[ -L "\$ROOT\/previous" \] && \[ ! -d "\$ROOT\/previous" \]; then\n\s*fail "the previous symlink points at a release that no longer exists"/, + Windows: + /\$prev -and -not \(Test-Path -LiteralPath \(Join-Path \$Root "releases\\\$prev"\) -PathType Container\)\) \{\n\s*Fail "previous\.txt points at a release that no longer exists"/, + }, + }, + { + // SELF_HOST.md -> "Definition of done" says the service definition carries + // no credential, and nothing checked it: a plist or unit carrying + // `DORMOUSE_SETUP_PASSWORD` passed `verify`. The search is over names the + // installer knows from docs/specs/security-remote.md -> "Credentials at + // rest"; the enrollment offer is exempted by its `_FILE` suffix, since the + // definition legitimately carries that path. + // + // Anchored on the search expression rather than on the message: the + // message is prose and the expression is the control. + rule: 'Credentials at rest — manage verify searches the service definition for a credential name', + patterns: { + macOS: + /grep -qE 'DORMOUSE_SETUP_PASSWORD\|DORMOUSE_VAPID_PRIVATE_KEY\|DORMOUSE_ENROLL_TOKEN\[\^_\]' \\\n\s*"\$PLIST" "\$ROOT\/bin\/run-relay"/, + Linux: + /grep -qE 'DORMOUSE_SETUP_PASSWORD\|DORMOUSE_VAPID_PRIVATE_KEY\|DORMOUSE_ENROLL_TOKEN\[\^_\]' \\\n\s*"\$UNIT_FILE" "\$ROOT\/bin\/run-relay"/, + Windows: + /\(\("\$taskXml" \+ "`n" \+ \$wrapperText\) -match 'DORMOUSE_SETUP_PASSWORD\|DORMOUSE_VAPID_PRIVATE_KEY\|DORMOUSE_ENROLL_TOKEN\(\?!_FILE\)'\)/, + }, + }, { rule: 'Network posture — the installer refuses to rewrite a mismatched DORMOUSE_ORIGIN', patterns: { diff --git a/scripts/spec-word-budgets.json b/scripts/spec-word-budgets.json index ac02de818..dd665da61 100644 --- a/scripts/spec-word-budgets.json +++ b/scripts/spec-word-budgets.json @@ -1,7 +1,7 @@ { "AGENTS.md": 3400, "SECURITY.md": 200, - "SELF_HOST.md": 6050, + "SELF_HOST.md": 6100, "docs/specs/alert.md": 6850, "docs/specs/auto-update.md": 1150, "docs/specs/deploy.md": 1850, From 5c1148884c42bbda76729c281c68e6b34821ecb5 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sun, 20 Sep 2026 07:54:18 -0700 Subject: [PATCH 3/5] Fix the remaining security audit review findings --- deploy/local/install-linux.sh | 20 +++++++++++++++---- deploy/local/install-macos.sh | 20 +++++++++++++++---- deploy/local/install-windows.ps1 | 28 +++++++++++++-------------- hosted/server/policy-constants.ts | 14 ++++++++++++++ hosted/server/policy.ts | 9 ++------- hosted/src/App.tsx | 2 +- scripts/deploy-lint.mjs | 32 ++++++++++++++++++++++++++++--- 7 files changed, 92 insertions(+), 33 deletions(-) create mode 100644 hosted/server/policy-constants.ts diff --git a/deploy/local/install-linux.sh b/deploy/local/install-linux.sh index 19da74748..cd0294730 100755 --- a/deploy/local/install-linux.sh +++ b/deploy/local/install-linux.sh @@ -1204,18 +1204,30 @@ cmd_verify() { # and nothing else secret (docs/specs/security-remote.md -> "Credentials at # rest"). A name, not a value: the installer supplies none of these, so one # appearing means a hand-edit or a regression put a credential where any - # process that can read the definition can read it. - if grep -qE 'DORMOUSE_SETUP_PASSWORD|DORMOUSE_VAPID_PRIVATE_KEY|DORMOUSE_ENROLL_TOKEN[^_]' \ + # process that can read the definition can read it. The trailing alternative + # is the same name at end of line: `[^_]` alone exempts a bare + # `export DORMOUSE_ENROLL_TOKEN` that inherits its value from the + # environment, which the Windows `(?!_FILE)` lookahead catches. + # + # `grep -q` exits 2 on a file it cannot open, which is neither a match nor a + # miss, so both searches report on definition_read rather than green-ticking + # a definition nothing read. + local definition_read=0 + if [ -r "$UNIT_FILE" ] && [ -r "$ROOT/bin/run-relay" ]; then definition_read=1; fi + if [ "$definition_read" = 0 ]; then + fail "the unit or bin/run-relay could not be read — it was searched for neither a credential nor the source checkout" + elif grep -qE 'DORMOUSE_SETUP_PASSWORD|DORMOUSE_VAPID_PRIVATE_KEY|DORMOUSE_ENROLL_TOKEN[^_]|DORMOUSE_ENROLL_TOKEN$' \ "$UNIT_FILE" "$ROOT/bin/run-relay" 2>/dev/null; then fail "the unit or wrapper names a credential — it must carry only paths" else pass "the service definition names no credential" fi - # The release must not depend on the source checkout. + # The release must not depend on the source checkout. Unreadable inputs have + # already failed above, so this search stays silent rather than failing twice. local src src="$(release_field source_checkout || echo '')" - if [ -n "$src" ]; then + if [ -n "$src" ] && [ "$definition_read" = 1 ]; then if grep -q "$src" "$UNIT_FILE" 2>/dev/null || grep -q "$src" "$ROOT/bin/run-relay" 2>/dev/null; then fail "the unit or wrapper references the source checkout ($src)" else diff --git a/deploy/local/install-macos.sh b/deploy/local/install-macos.sh index 6c7897cae..77804b7bb 100755 --- a/deploy/local/install-macos.sh +++ b/deploy/local/install-macos.sh @@ -970,18 +970,30 @@ cmd_verify() { # and nothing else secret (docs/specs/security-remote.md -> "Credentials at # rest"). A name, not a value: the installer supplies none of these, so one # appearing means a hand-edit or a regression put a credential where any - # process that can read the definition can read it. - if grep -qE 'DORMOUSE_SETUP_PASSWORD|DORMOUSE_VAPID_PRIVATE_KEY|DORMOUSE_ENROLL_TOKEN[^_]' \ + # process that can read the definition can read it. The trailing alternative + # is the same name at end of line: `[^_]` alone exempts a bare + # `export DORMOUSE_ENROLL_TOKEN` that inherits its value from the + # environment, which the Windows `(?!_FILE)` lookahead catches. + # + # `grep -q` exits 2 on a file it cannot open, which is neither a match nor a + # miss, so both searches report on definition_read rather than green-ticking + # a definition nothing read. + local definition_read=0 + if [ -r "$PLIST" ] && [ -r "$ROOT/bin/run-relay" ]; then definition_read=1; fi + if [ "$definition_read" = 0 ]; then + fail "the LaunchAgent or bin/run-relay could not be read — it was searched for neither a credential nor the source checkout" + elif grep -qE 'DORMOUSE_SETUP_PASSWORD|DORMOUSE_VAPID_PRIVATE_KEY|DORMOUSE_ENROLL_TOKEN[^_]|DORMOUSE_ENROLL_TOKEN$' \ "$PLIST" "$ROOT/bin/run-relay" 2>/dev/null; then fail "the LaunchAgent or wrapper names a credential — it must carry only paths" else pass "the service definition names no credential" fi - # The release must not depend on the source checkout. + # The release must not depend on the source checkout. Unreadable inputs have + # already failed above, so this search stays silent rather than failing twice. local src src="$(release_field source_checkout || echo '')" - if [ -n "$src" ]; then + if [ -n "$src" ] && [ "$definition_read" = 1 ]; then if grep -q "$src" "$PLIST" 2>/dev/null || grep -q "$src" "$ROOT/bin/run-relay" 2>/dev/null; then fail "the LaunchAgent or wrapper references the source checkout ($src)" else diff --git a/deploy/local/install-windows.ps1 b/deploy/local/install-windows.ps1 index 8bf3c60b9..d4dbe4ab4 100644 --- a/deploy/local/install-windows.ps1 +++ b/deploy/local/install-windows.ps1 @@ -1474,14 +1474,17 @@ function Invoke-Verify { # CIM round trip into the Task Scheduler service -- the priciest call in this # whole path -- so it is made once here rather than once per check. # - # It can also come back $null: CIM blocked by policy, the Task Scheduler - # service momentarily unavailable, or the task unregistered between Get-Task - # and here. Treat that as its own outcome and fail rather than verifying a - # definition we could not inspect. + # Either can come back unread. The export is $null when CIM is blocked by + # policy, when the Task Scheduler service is momentarily unavailable, or when + # the task is unregistered between Get-Task and here; the wrapper text stays + # '' when the file is missing. "$null" is the empty string, so neither search + # can fire on one -- both report on $definitionRead instead, and a definition + # nothing inspected fails rather than green-ticking twice. $taskXml = Export-ScheduledTask -TaskName $LABEL -TaskPath $TASK_PATH -ErrorAction SilentlyContinue $wrapper = Join-Path $Root 'bin\run-relay.ps1' $wrapperText = '' if (Test-Path -LiteralPath $wrapper -PathType Leaf) { $wrapperText = [IO.File]::ReadAllText($wrapper) } + $definitionRead = [bool]$taskXml -and [bool]$wrapperText $task = Get-Task if ($task) { @@ -1522,12 +1525,6 @@ function Invoke-Verify { } else { Fail "bin\run-relay.ps1 is missing or has no supervision loop" } - - # The only consumer left is the source-checkout search near the end, so an - # export that failed means that search covered the wrapper alone. Reported - # here, inside the registered-task branch, so an unregistered task fails - # once rather than twice. - if (-not $taskXml) { Fail "the task definition could not be exported -- it was not searched for the source checkout" } } else { Fail "Scheduled Task $TASK_PATH$LABEL is not registered" } @@ -1700,20 +1697,23 @@ function Invoke-Verify { # rest"). A name, not a value: the installer supplies none of these, so one # appearing means a hand-edit or a regression put a credential where any # process that can read the definition can read it. - if (("$taskXml" + "`n" + $wrapperText) -match 'DORMOUSE_SETUP_PASSWORD|DORMOUSE_VAPID_PRIVATE_KEY|DORMOUSE_ENROLL_TOKEN(?!_FILE)') { + if (-not $definitionRead) { + Fail "the task definition or bin\run-relay.ps1 could not be read -- it was searched for neither a credential nor the source checkout" + } elseif (("$taskXml" + "`n" + $wrapperText) -match 'DORMOUSE_SETUP_PASSWORD|DORMOUSE_VAPID_PRIVATE_KEY|DORMOUSE_ENROLL_TOKEN(?!_FILE)') { Fail "the Scheduled Task or wrapper names a credential -- it must carry only paths" } else { Pass "the service definition names no credential" } - # The release must not depend on the source checkout. + # The release must not depend on the source checkout. Unreadable inputs have + # already failed above, so this search stays silent rather than failing twice. $src = Get-ReleaseField 'source_checkout' - if ($src) { + if ($src -and $definitionRead) { $refs = $false if ($wrapperText -match [regex]::Escape($src)) { $refs = $true } if ($taskXml -match [regex]::Escape($src)) { $refs = $true } if ($refs) { Fail "the Scheduled Task or wrapper references the source checkout ($src)" } - elseif ($taskXml) { Pass "the installed service does not reference the source checkout" } + else { Pass "the installed service does not reference the source checkout" } } Write-Host "" diff --git a/hosted/server/policy-constants.ts b/hosted/server/policy-constants.ts new file mode 100644 index 000000000..ac78d6178 --- /dev/null +++ b/hosted/server/policy-constants.ts @@ -0,0 +1,14 @@ +// Policy values the Worker and the frontend bundle both need. This module +// imports nothing, for the reason `./providers.js` does not either: `./policy` +// reaches the OAuth client secrets through `providerBindings`, and +// docs/specs/security-hosted.md -> "Origin boundary" has an auditor inspect the +// frontend import graph. The account screen takes its constants from here so +// that graph never walks back to the module holding the secrets. + +// How long after signing in a login still counts as recent enough to connect a +// provider. The packed adapter enforces it as Better Auth's `session.freshAge` +// and the account screen gates its buttons on the same window, so the two would +// desync silently on a pgstencil bump: a UI that offers "connect" for a login +// the server has already stopped accepting 302s to /login?error=. One value, +// and `hosted/server/tests/policy.test.ts` pins it to what the adapter builds. +export const LOGIN_FRESH_AGE_MS = 10 * 60 * 1000; diff --git a/hosted/server/policy.ts b/hosted/server/policy.ts index ecb698a4a..ce0ef9b8b 100644 --- a/hosted/server/policy.ts +++ b/hosted/server/policy.ts @@ -1,6 +1,7 @@ import type { AuthAppOptions } from "@pgstencil/auth/better-auth"; import { providerIds } from "./providers.js"; import type { ProviderId } from "./providers.js"; +import { LOGIN_FRESH_AGE_MS } from "./policy-constants.js"; export const authPolicy = { appName: "Dormouse Hosted", @@ -16,13 +17,7 @@ export const authPolicy = { // plain-node deploy scripts import without this module's dependencies. export { providerIds, type ProviderId }; -// How long after signing in a login still counts as recent enough to connect a -// provider. The packed adapter enforces it as Better Auth's `session.freshAge` -// and the account screen gates its buttons on the same window, so the two would -// desync silently on a pgstencil bump: a UI that offers "connect" for a login -// the server has already stopped accepting 302s to /login?error=. One value, -// and `hosted/server/tests/policy.test.ts` pins it to what the adapter builds. -export const LOGIN_FRESH_AGE_MS = 10 * 60 * 1000; +export { LOGIN_FRESH_AGE_MS } from "./policy-constants.js"; // Only an explicit deployment allowlist enables a provider; stale secrets do not. export function providerBindings(env: Record) { diff --git a/hosted/src/App.tsx b/hosted/src/App.tsx index a428b1454..0b836490c 100644 --- a/hosted/src/App.tsx +++ b/hosted/src/App.tsx @@ -16,7 +16,7 @@ import { type Provider, type Session, } from "./api"; -import { LOGIN_FRESH_AGE_MS } from "../server/policy"; +import { LOGIN_FRESH_AGE_MS } from "../server/policy-constants"; export function App() { const [session, setSession] = useState(null); diff --git a/scripts/deploy-lint.mjs b/scripts/deploy-lint.mjs index 52e3d0635..54c354d73 100644 --- a/scripts/deploy-lint.mjs +++ b/scripts/deploy-lint.mjs @@ -252,17 +252,43 @@ export const RULES = [ // definition legitimately carries that path. // // Anchored on the search expression rather than on the message: the - // message is prose and the expression is the control. + // message is prose and the expression is the control. The unix expression + // carries the enrollment token twice because `[^_]` needs a character + // after the name: without the end-anchored alternative a bare + // `export DORMOUSE_ENROLL_TOKEN` passed on both shells while the Windows + // lookahead failed it, and a control present in one installer and absent + // from another is what this lint exists to catch. rule: 'Credentials at rest — manage verify searches the service definition for a credential name', patterns: { macOS: - /grep -qE 'DORMOUSE_SETUP_PASSWORD\|DORMOUSE_VAPID_PRIVATE_KEY\|DORMOUSE_ENROLL_TOKEN\[\^_\]' \\\n\s*"\$PLIST" "\$ROOT\/bin\/run-relay"/, + /grep -qE 'DORMOUSE_SETUP_PASSWORD\|DORMOUSE_VAPID_PRIVATE_KEY\|DORMOUSE_ENROLL_TOKEN\[\^_\]\|DORMOUSE_ENROLL_TOKEN\$' \\\n\s*"\$PLIST" "\$ROOT\/bin\/run-relay"/, Linux: - /grep -qE 'DORMOUSE_SETUP_PASSWORD\|DORMOUSE_VAPID_PRIVATE_KEY\|DORMOUSE_ENROLL_TOKEN\[\^_\]' \\\n\s*"\$UNIT_FILE" "\$ROOT\/bin\/run-relay"/, + /grep -qE 'DORMOUSE_SETUP_PASSWORD\|DORMOUSE_VAPID_PRIVATE_KEY\|DORMOUSE_ENROLL_TOKEN\[\^_\]\|DORMOUSE_ENROLL_TOKEN\$' \\\n\s*"\$UNIT_FILE" "\$ROOT\/bin\/run-relay"/, Windows: /\(\("\$taskXml" \+ "`n" \+ \$wrapperText\) -match 'DORMOUSE_SETUP_PASSWORD\|DORMOUSE_VAPID_PRIVATE_KEY\|DORMOUSE_ENROLL_TOKEN\(\?!_FILE\)'\)/, }, }, + { + // SELF_HOST.md -> "Definition of done": "a definition it cannot read at all + // fails rather than passes". Both searches above read the service + // definition and the `run-relay` wrapper, and on every platform an + // unreadable input reported a pass — `grep -q` exits 2 on a file it cannot + // open, and PowerShell's `"$null"` is the empty string, so neither could + // match and both took the green branch. + // + // Condition and verdict as one span, per platform: the message alone could + // survive the guard being deleted, and the guard alone could survive it + // reporting a pass. + rule: 'Definition of done — manage verify fails a definition it could not read', + patterns: { + macOS: + /if \[ "\$definition_read" = 0 \]; then\n\s*fail "the LaunchAgent or bin\/run-relay could not be read/, + Linux: + /if \[ "\$definition_read" = 0 \]; then\n\s*fail "the unit or bin\/run-relay could not be read/, + Windows: + /if \(-not \$definitionRead\) \{\n\s*Fail "the task definition or bin\\run-relay\.ps1 could not be read/, + }, + }, { rule: 'Network posture — the installer refuses to rewrite a mismatched DORMOUSE_ORIGIN', patterns: { From 50e849897c55618f48ed6afe7a16a44720b246a5 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sun, 20 Sep 2026 08:13:07 -0700 Subject: [PATCH 4/5] Align credential-name checks across installers --- deploy/local/install-linux.sh | 12 ++++++------ deploy/local/install-macos.sh | 12 ++++++------ hosted/server/policy.ts | 1 - scripts/deploy-lint.mjs | 23 +++++++++++++++-------- 4 files changed, 27 insertions(+), 21 deletions(-) diff --git a/deploy/local/install-linux.sh b/deploy/local/install-linux.sh index cd0294730..71493b8f9 100755 --- a/deploy/local/install-linux.sh +++ b/deploy/local/install-linux.sh @@ -1204,10 +1204,9 @@ cmd_verify() { # and nothing else secret (docs/specs/security-remote.md -> "Credentials at # rest"). A name, not a value: the installer supplies none of these, so one # appearing means a hand-edit or a regression put a credential where any - # process that can read the definition can read it. The trailing alternative - # is the same name at end of line: `[^_]` alone exempts a bare - # `export DORMOUSE_ENROLL_TOKEN` that inherits its value from the - # environment, which the Windows `(?!_FILE)` lookahead catches. + # process that can read the definition can read it. Extracting names before + # filtering exempts exactly DORMOUSE_ENROLL_TOKEN_FILE; a bare token or any + # other suffix remains a finding on every platform. # # `grep -q` exits 2 on a file it cannot open, which is neither a match nor a # miss, so both searches report on definition_read rather than green-ticking @@ -1216,8 +1215,9 @@ cmd_verify() { if [ -r "$UNIT_FILE" ] && [ -r "$ROOT/bin/run-relay" ]; then definition_read=1; fi if [ "$definition_read" = 0 ]; then fail "the unit or bin/run-relay could not be read — it was searched for neither a credential nor the source checkout" - elif grep -qE 'DORMOUSE_SETUP_PASSWORD|DORMOUSE_VAPID_PRIVATE_KEY|DORMOUSE_ENROLL_TOKEN[^_]|DORMOUSE_ENROLL_TOKEN$' \ - "$UNIT_FILE" "$ROOT/bin/run-relay" 2>/dev/null; then + elif grep -hEo 'DORMOUSE_SETUP_PASSWORD|DORMOUSE_VAPID_PRIVATE_KEY|DORMOUSE_ENROLL_TOKEN(_[[:alnum:]_]+)?' \ + "$UNIT_FILE" "$ROOT/bin/run-relay" 2>/dev/null |\ + grep -qvx 'DORMOUSE_ENROLL_TOKEN_FILE'; then fail "the unit or wrapper names a credential — it must carry only paths" else pass "the service definition names no credential" diff --git a/deploy/local/install-macos.sh b/deploy/local/install-macos.sh index 77804b7bb..6016ba6b3 100755 --- a/deploy/local/install-macos.sh +++ b/deploy/local/install-macos.sh @@ -970,10 +970,9 @@ cmd_verify() { # and nothing else secret (docs/specs/security-remote.md -> "Credentials at # rest"). A name, not a value: the installer supplies none of these, so one # appearing means a hand-edit or a regression put a credential where any - # process that can read the definition can read it. The trailing alternative - # is the same name at end of line: `[^_]` alone exempts a bare - # `export DORMOUSE_ENROLL_TOKEN` that inherits its value from the - # environment, which the Windows `(?!_FILE)` lookahead catches. + # process that can read the definition can read it. Extracting names before + # filtering exempts exactly DORMOUSE_ENROLL_TOKEN_FILE; a bare token or any + # other suffix remains a finding on every platform. # # `grep -q` exits 2 on a file it cannot open, which is neither a match nor a # miss, so both searches report on definition_read rather than green-ticking @@ -982,8 +981,9 @@ cmd_verify() { if [ -r "$PLIST" ] && [ -r "$ROOT/bin/run-relay" ]; then definition_read=1; fi if [ "$definition_read" = 0 ]; then fail "the LaunchAgent or bin/run-relay could not be read — it was searched for neither a credential nor the source checkout" - elif grep -qE 'DORMOUSE_SETUP_PASSWORD|DORMOUSE_VAPID_PRIVATE_KEY|DORMOUSE_ENROLL_TOKEN[^_]|DORMOUSE_ENROLL_TOKEN$' \ - "$PLIST" "$ROOT/bin/run-relay" 2>/dev/null; then + elif grep -hEo 'DORMOUSE_SETUP_PASSWORD|DORMOUSE_VAPID_PRIVATE_KEY|DORMOUSE_ENROLL_TOKEN(_[[:alnum:]_]+)?' \ + "$PLIST" "$ROOT/bin/run-relay" 2>/dev/null |\ + grep -qvx 'DORMOUSE_ENROLL_TOKEN_FILE'; then fail "the LaunchAgent or wrapper names a credential — it must carry only paths" else pass "the service definition names no credential" diff --git a/hosted/server/policy.ts b/hosted/server/policy.ts index ce0ef9b8b..647d5f0c2 100644 --- a/hosted/server/policy.ts +++ b/hosted/server/policy.ts @@ -1,7 +1,6 @@ import type { AuthAppOptions } from "@pgstencil/auth/better-auth"; import { providerIds } from "./providers.js"; import type { ProviderId } from "./providers.js"; -import { LOGIN_FRESH_AGE_MS } from "./policy-constants.js"; export const authPolicy = { appName: "Dormouse Hosted", diff --git a/scripts/deploy-lint.mjs b/scripts/deploy-lint.mjs index 54c354d73..369e73e2c 100644 --- a/scripts/deploy-lint.mjs +++ b/scripts/deploy-lint.mjs @@ -252,22 +252,29 @@ export const RULES = [ // definition legitimately carries that path. // // Anchored on the search expression rather than on the message: the - // message is prose and the expression is the control. The unix expression - // carries the enrollment token twice because `[^_]` needs a character - // after the name: without the end-anchored alternative a bare - // `export DORMOUSE_ENROLL_TOKEN` passed on both shells while the Windows - // lookahead failed it, and a control present in one installer and absent - // from another is what this lint exists to catch. + // message is prose and the expression is the control. Unix extracts each + // known credential name before filtering the one allowed path variable. rule: 'Credentials at rest — manage verify searches the service definition for a credential name', patterns: { macOS: - /grep -qE 'DORMOUSE_SETUP_PASSWORD\|DORMOUSE_VAPID_PRIVATE_KEY\|DORMOUSE_ENROLL_TOKEN\[\^_\]\|DORMOUSE_ENROLL_TOKEN\$' \\\n\s*"\$PLIST" "\$ROOT\/bin\/run-relay"/, + /grep -hEo 'DORMOUSE_SETUP_PASSWORD\|DORMOUSE_VAPID_PRIVATE_KEY\|DORMOUSE_ENROLL_TOKEN\(_\[\[:alnum:\]_\]\+\)\?'/, Linux: - /grep -qE 'DORMOUSE_SETUP_PASSWORD\|DORMOUSE_VAPID_PRIVATE_KEY\|DORMOUSE_ENROLL_TOKEN\[\^_\]\|DORMOUSE_ENROLL_TOKEN\$' \\\n\s*"\$UNIT_FILE" "\$ROOT\/bin\/run-relay"/, + /grep -hEo 'DORMOUSE_SETUP_PASSWORD\|DORMOUSE_VAPID_PRIVATE_KEY\|DORMOUSE_ENROLL_TOKEN\(_\[\[:alnum:\]_\]\+\)\?'/, Windows: /\(\("\$taskXml" \+ "`n" \+ \$wrapperText\) -match 'DORMOUSE_SETUP_PASSWORD\|DORMOUSE_VAPID_PRIVATE_KEY\|DORMOUSE_ENROLL_TOKEN\(\?!_FILE\)'\)/, }, }, + { + // The service definition legitimately carries DORMOUSE_ENROLL_TOKEN_FILE, + // and no other name in that namespace. Pin the exact Unix filter alongside + // the equivalent Windows negative lookahead so the platforms cannot drift. + rule: 'Credentials at rest — only the enrollment token file name is exempt', + patterns: { + macOS: /grep -qvx 'DORMOUSE_ENROLL_TOKEN_FILE'/, + Linux: /grep -qvx 'DORMOUSE_ENROLL_TOKEN_FILE'/, + Windows: /DORMOUSE_ENROLL_TOKEN\(\?!_FILE\)/, + }, + }, { // SELF_HOST.md -> "Definition of done": "a definition it cannot read at all // fails rather than passes". Both searches above read the service From eeff507f5d31e1b3708636d55e0738d5fc4f3920 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sun, 20 Sep 2026 08:26:04 -0700 Subject: [PATCH 5/5] Word-bound the Windows enrollment-token exemption so _FILE_PATH is a finding there too The Unix search exempts exactly DORMOUSE_ENROLL_TOKEN_FILE; the Windows lookahead was not end-anchored, so a name extending _FILE slipped through on that platform alone. (?!_FILE\b) closes it, and deploy-lint pins the bounded form in both rules. Co-Authored-By: Claude Fable 5.1 --- deploy/local/install-windows.ps1 | 2 +- scripts/deploy-lint.mjs | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/deploy/local/install-windows.ps1 b/deploy/local/install-windows.ps1 index d4dbe4ab4..683933922 100644 --- a/deploy/local/install-windows.ps1 +++ b/deploy/local/install-windows.ps1 @@ -1699,7 +1699,7 @@ function Invoke-Verify { # process that can read the definition can read it. if (-not $definitionRead) { Fail "the task definition or bin\run-relay.ps1 could not be read -- it was searched for neither a credential nor the source checkout" - } elseif (("$taskXml" + "`n" + $wrapperText) -match 'DORMOUSE_SETUP_PASSWORD|DORMOUSE_VAPID_PRIVATE_KEY|DORMOUSE_ENROLL_TOKEN(?!_FILE)') { + } elseif (("$taskXml" + "`n" + $wrapperText) -match 'DORMOUSE_SETUP_PASSWORD|DORMOUSE_VAPID_PRIVATE_KEY|DORMOUSE_ENROLL_TOKEN(?!_FILE\b)') { Fail "the Scheduled Task or wrapper names a credential -- it must carry only paths" } else { Pass "the service definition names no credential" diff --git a/scripts/deploy-lint.mjs b/scripts/deploy-lint.mjs index 369e73e2c..00ecb24a9 100644 --- a/scripts/deploy-lint.mjs +++ b/scripts/deploy-lint.mjs @@ -261,18 +261,19 @@ export const RULES = [ Linux: /grep -hEo 'DORMOUSE_SETUP_PASSWORD\|DORMOUSE_VAPID_PRIVATE_KEY\|DORMOUSE_ENROLL_TOKEN\(_\[\[:alnum:\]_\]\+\)\?'/, Windows: - /\(\("\$taskXml" \+ "`n" \+ \$wrapperText\) -match 'DORMOUSE_SETUP_PASSWORD\|DORMOUSE_VAPID_PRIVATE_KEY\|DORMOUSE_ENROLL_TOKEN\(\?!_FILE\)'\)/, + /\(\("\$taskXml" \+ "`n" \+ \$wrapperText\) -match 'DORMOUSE_SETUP_PASSWORD\|DORMOUSE_VAPID_PRIVATE_KEY\|DORMOUSE_ENROLL_TOKEN\(\?!_FILE\\b\)'\)/, }, }, { // The service definition legitimately carries DORMOUSE_ENROLL_TOKEN_FILE, // and no other name in that namespace. Pin the exact Unix filter alongside - // the equivalent Windows negative lookahead so the platforms cannot drift. + // the equivalent Windows negative lookahead (word-bounded, so a name that + // merely extends _FILE is a finding there too) so the platforms cannot drift. rule: 'Credentials at rest — only the enrollment token file name is exempt', patterns: { macOS: /grep -qvx 'DORMOUSE_ENROLL_TOKEN_FILE'/, Linux: /grep -qvx 'DORMOUSE_ENROLL_TOKEN_FILE'/, - Windows: /DORMOUSE_ENROLL_TOKEN\(\?!_FILE\)/, + Windows: /DORMOUSE_ENROLL_TOKEN\(\?!_FILE\\b\)/, }, }, {