Correct the security specs and audit prompts; close two manage-verify gaps - #715
Conversation
Deploying mouseterm with
|
| Latest commit: |
eeff507
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://bb4e0887.mouseterm.pages.dev |
| Branch Preview URL: | https://specs-security-audit.mouseterm.pages.dev |
dormouse-bot
left a comment
dormouse-bot
left a comment
There was a problem hiding this comment.
Feedback on work in progress, not a merge verdict — mark it ready when you want the full review.
Two issues in the new manage verify credential check, and one note on the Hosted constant, inline. The spec compression itself reads correctly where I sampled it against the rationale files and the code.
Standalone Smoketest is red on this head; the run is still in progress, so its log isn't readable yet.
dormouse-bot
left a comment
dormouse-bot
left a comment
There was a problem hiding this comment.
Feedback on work in progress, not a merge verdict — mark it ready when you want the full review.
f81c08fa closes all three findings from the previous round, and I've resolved those threads. Two new points inline: a dead import left behind by the policy-constants split, and one residue of the unix/Windows divergence this commit set out to close.
dormouse-bot
left a comment
dormouse-bot
left a comment
There was a problem hiding this comment.
Feedback on work in progress, not a merge verdict — mark it ready when you want the full review.
7155af4f closes both findings from the last round: extract-then-filter makes unix flag DORMOUSE_ENROLL_TOKEN_VALUE and the bare export exactly as Windows does, and the dead policy.ts import is gone. I've resolved those threads. One new point inline, on the comment the commit writes alongside the fix.
dormouse-bot
left a comment
dormouse-bot
left a comment
There was a problem hiding this comment.
Two findings inline, plus two notes.
The self-test one is the one worth acting on: removing check 15 from spec-lint.mjs entirely leaves spec-lint-selftest.mjs green, so the new case records a hold for check 11 rather than for the rule it names. The other is a citation in hosted/src/style.css pointing at a rule this PR deletes from hosted.md.
node scripts/deploy-lint-selftest.mjs on this head reports 124 load-bearing checks, not the 118 in the PR description — the two rules added after the first commit account for the difference.
#700 is a separate draft off main touching docs/specs/security-audit.md and .github/workflows/security-audit.yaml. This PR leaves that spec at exactly its 1,900-word budget, so whichever lands second needs a --ratchet in the same commit.
| selftest.withAppended(target, text, `${name}\n planting this in ${target} stays green — spec-lint cannot see it`); | ||
| } | ||
|
|
||
| selftest.withMutation( |
There was a problem hiding this comment.
This case holds whether or not check 15 exists. withMutation asserts only that the lint went red, and removing SELF_HOST.rationale.md independently trips check 11 on each of SELF_HOST.md's eight (rationale) markers — so the red comes from check 11, not from the rule this case names.
I deleted check 15's loop from spec-lint.mjs and reran with the rationale removed: still exit 1, reporting the eight SELF_HOST.md:NNN: "(rationale)" marker, but SELF_HOST.rationale.md does not exist lines and no check-15 diagnostic. AGENTS.md -> "Specs": "A rule added to one of these lints without its self-test case is not enforced — it is a claim that something is checked."
Closing it means asserting the diagnostic rather than the exit code, which needs a small lint-kit.mjs change too: there is no mutation-plus-expected-output variant (withAppendedOutput requires the lint to pass), and runLint captures only stdout while spec-lint writes problems to stderr. Not posted as a one-click suggestion for that reason — happy to push it if you want.
| /* | ||
| * 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. | ||
| */ |
There was a problem hiding this comment.
docs/specs/hosted.md -> Interface no longer states that rule. This PR replaces the bullet's tail with "Its type scale and touch sizing are in hosted/src/style.css", and "single-column" now appears nowhere under docs/ — so the citation names a rule that does not exist, and the layout constraint is left owned by nobody.
hosted.md is at 1,048 of its 1,050 words, so moving the phrase back there is not free. Keeping it here, where hosted.md now points, is the cheaper direction:
| /* | |
| * 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. | |
| */ | |
| /* | |
| * The Hosted type scale and touch sizing, which docs/specs/hosted.md -> | |
| * Interface points here for: 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. | |
| */ |
| # 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 (-not $definitionRead) { |
There was a problem hiding this comment.
Moving the guard out of the if ($task) branch reverses what the deleted one was placed for — its comment said so: "Reported here, inside the registered-task branch, so an unregistered task fails once rather than twice."
An unregistered task now fails twice. Get-Task returns nothing, so Fail "Scheduled Task $TASK_PATH$LABEL is not registered" fires; Export-ScheduledTask returns $null, so $definitionRead is false and this fires too. A missing bin\run-relay.ps1 doubles the same way against "bin\run-relay.ps1 is missing or has no supervision loop".
Separately, $definitionRead is all-or-nothing, so a readable task definition carrying DORMOUSE_SETUP_PASSWORD beside a missing wrapper reports "could not be read" rather than "names a credential". verify exits nonzero either way, so that half is message quality, not a missed finding.
No one-click suggestion: scripts/deploy-lint.mjs pins if \(-not \$definitionRead\) \{\n\s*Fail "the task definition... literally, so re-nesting the guard needs that rule edited in the same commit.
…ionale 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 <noreply@anthropic.com>
…o values 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 <noreply@anthropic.com>
…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 <noreply@anthropic.com>
3c92b91 to
eeff507
Compare
Resolves the Orchestration statement against #715's compressed form, keeping this PR's widening to a delegating domain, and re-ratchets the two budgets the conflict spanned. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2 participants
Spec audit, group 7 (the
security*.mdspecs,SECURITY.md,SELF_HOST.md, the.github/audit/prompts). Stacked on #714 (the spec-audit PRs merge in order #708 → #715); independent of the alert series. NoFAIL IFcondition was deleted; several were corrected.Call-outs
security-audit.md's DomainsFAIL IFsaid.github/audit//.vscode/must be outside every consumer of the workflow-audit window, which could never fire on the failure its own rationale names (one consumer widened without the others). Now any.manage verifygaps: a danglingpreviouspointer failed only on Linux (macOS/Windows green-ticked a pruned rollback target), and no platform inspected the service definition or wrapper for a credential. Both are now checks in all three installers withdeploy-lintrules and self-test mutations (112 → 118 checks). The Windows branch is lint-read only — no PowerShell on this machine.supply-chainaudit prompt never used$AUDIT_PAT, so its admin-scopedFAIL IFwould readnulland record a false FAIL; fixed. Theapplication-securityprompt now carries the Hosted guidance its 11 claimed rules need.Also:
recovery.jsonis 0600/0700, not "written at the umask" (Windows is the real residual); the ten-minute Bash cap is now tied toBASH_DEFAULT_TIMEOUT_MS;pull_request_targetscoped to anon:trigger; the monotone-upward issue-title rule stated; two unpinned claims now name their tests;SECURITY.mdpoints at the Domains table instead of listing five of six audited specs; spec-lint gains check 17 (FAIL IFonly insecurity*.md) and check 15 now covers root specs, both self-tested;SELF_HOST.mdgets its rationale file and loses ≈100 words of restatedsecurity-remote.mdrules; the Hosted freshness window is one constant pinned against the server gate.Budgets: SELF_HOST 6200→6100, security-remote 5850→5750, security-supply-chain 1250→1150.
Deferred (owner decisions or larger refactors): one release-pointer mechanism, one sentinel predicate for the audit fragments, one report merge,
manage statusvsverify, the fixedPORTkey, the generate-deps override tables.🤖 Generated with Claude Code