From cd3e0465dc17aead53dffeaef189e3e016942b49 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Mon, 21 Sep 2026 16:54:12 +0200 Subject: [PATCH] fix(setup): stop the update skill reporting the dropped lychee domains as drift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Check 4 of `setup-isolated-setup-update` told the agent that the dogfooded `allowedDomains` default carries eleven wildcard link-target hosts (`*.apache.org`, `*.nist.gov`, `lychee.cli.rs`, …) and that "without these, lychee fails the PR-blocking prek check locally on first run". Both claims stopped being true in #501, which moved the lychee hook to `offline = true` and dropped those hosts from `.claude/settings.json` precisely because the hook no longer reaches them. `docs/setup/secure-agent-setup.md` was updated in that PR; this skill was not, so a drift report run against current settings reports eleven correctly-absent domains as missing defaults and walks the adopter into re-adding dead allowlist entries. Rewrite the bullet pair to the post-#501 reality: the default allows only the two hosts prek needs to bootstrap rustup and `cargo install` lychee, the removed hosts are dead weight to *drop* if still present, and `enableWeakerNetworkIsolation` stays for native-TLS CLI tools (`gh` / `gcloud` / `terraform`) rather than for lychee. Add a guard so the prose cannot silently drift from the settings again: `tools/sandbox-lint/tests/test_doc_domain_consistency.py` parses both lists out of the skill and asserts the kept hosts are allowed and the dropped hosts are not. Re-adding `*.nist.gov` to the settings, or removing `static.rust-lang.org`, now fails the workspace pytest hook. Generated-by: Claude Opus 5 --- docs/mode-economics.md | 4 +- .../skills/isolated-setup-update/SKILL.md | 49 ++++++------ .../tests/test_doc_domain_consistency.py | 75 +++++++++++++++++++ 3 files changed, 103 insertions(+), 25 deletions(-) create mode 100644 tools/sandbox-lint/tests/test_doc_domain_consistency.py diff --git a/docs/mode-economics.md b/docs/mode-economics.md index 6b6e2df3..c5528716 100644 --- a/docs/mode-economics.md +++ b/docs/mode-economics.md @@ -92,7 +92,7 @@ special-token spellings counted as ordinary text. Coverage: **75 of 75 local `skills/*/SKILL.md` files**. External `source.md` redirects and harness symlinks are excluded. -Measurement manifest SHA-256: `4556129999018ce339eb0f7c8292e18a424d57716fadd8407f29d9c4a9decec3`. +Measurement manifest SHA-256: `1ae8680e2a68bddd8a2d9ce0a761b055cebac5ea74312ba2a05196b6f3ba3cd9`. | Skill file | Measured tokens | Source SHA-256 (first 16 characters) | |---|---:|---| @@ -161,7 +161,7 @@ Measurement manifest SHA-256: `4556129999018ce339eb0f7c8292e18a424d57716fadd8407 | [setup](../skills/setup/SKILL.md) | 8,724 | `82788542bb240309` | | [setup-isolated-setup-doctor](../skills/setup-isolated-setup-doctor/SKILL.md) | 7,950 | `3fa5d728fa080ed0` | | [setup-isolated-setup-install](../skills/setup-isolated-setup-install/SKILL.md) | 11,278 | `418ea9794077c6e9` | -| [setup-isolated-setup-update](../skills/setup-isolated-setup-update/SKILL.md) | 5,069 | `dd56cf376cd14f55` | +| [setup-isolated-setup-update](../skills/setup-isolated-setup-update/SKILL.md) | 5,111 | `96096e6ac10407d3` | | [setup-isolated-setup-verify](../skills/setup-isolated-setup-verify/SKILL.md) | 8,502 | `6f0a2594801bdf27` | | [setup-override-upstream](../skills/setup-override-upstream/SKILL.md) | 4,012 | `fb583feb56b7f77c` | | [setup-privacy-llm](../skills/setup-privacy-llm/SKILL.md) | 2,145 | `0e27b542a1656846` | diff --git a/plugins/magpie-setup/skills/isolated-setup-update/SKILL.md b/plugins/magpie-setup/skills/isolated-setup-update/SKILL.md index 4c899ce4..efed1abe 100644 --- a/plugins/magpie-setup/skills/isolated-setup-update/SKILL.md +++ b/plugins/magpie-setup/skills/isolated-setup-update/SKILL.md @@ -257,29 +257,32 @@ Walk each: the secure setup before the guard shipped. Report new entries the user does not have; do not auto-merge. - Two network-layer defaults landed with the `lychee` link-check - prek hook — surface both if the user's settings predate them - (both `sandbox.network.*`): - - - **Broadened `allowedDomains`.** The dogfooded default now - allows the curated set the framework's own docs and dev tools - reach — `*.crates.io` (so the rust `lychee` hook can - `cargo install` lychee), `*.apache.org`, `*.anthropic.com`, - `*.claude.com`, `*.mitre.org`, `*.nist.gov`, `*.github.io`, - `gist.github.com`, `astral.sh`, `json.schemastore.org`, - `lychee.cli.rs`, `sdkman.io`. Without these, lychee fails the - PR-blocking `prek` check locally on first run. - - **`enableWeakerNetworkIsolation: true`.** Required for - native-TLS CLI tools (lychee, and the same mechanism the - schema notes for `gh` / `gcloud` / `terraform`) to verify TLS - through the sandbox's TLS-terminating proxy — without it lychee - fails every external link with `failed to verify TLS - certificate`. **Surface the documented trade-off when - reporting it**: the schema warns it "reduces security — opens a - potential data-exfiltration vector through the trustd service," - so the user decides whether to enable it (the default ships it - on because the link check needs it). It is a no-op outside the - sandbox, e.g. in CI. + Two `sandbox.network.*` settings are worth a look while diffing + — but neither is a "missing default" to re-add: + + - **`allowedDomains` is deliberately narrow.** The dogfooded + default allows `*.crates.io` and `static.rust-lang.org`, the + only hosts prek needs to bootstrap a rustup toolchain and + `cargo install` the `lychee` link-check hook on first run. + The wildcard link-target hosts that once sat beside them + (`*.apache.org`, `*.anthropic.com`, `*.claude.com`, + `*.mitre.org`, `*.nist.gov`, `*.github.io`, `gist.github.com`, + `astral.sh`, `json.schemastore.org`, `lychee.cli.rs`, + `sdkman.io`) were dropped when the hook went offline + (`offline = true` in `.lychee.toml`): lychee no longer fetches + the URLs the docs link to, so it never reaches them. A settings + file without those hosts is current, not stale — report their + *presence* as dead weight to drop, never their absence as + drift. + - **`enableWeakerNetworkIsolation: true`.** It is not there for + lychee, which runs offline. It lets native-TLS CLI tools verify + TLS through the sandbox's TLS-terminating proxy — the mechanism + the schema notes for `gh` / `gcloud` / `terraform`. **Surface + the documented trade-off when reporting it**: the schema warns + it "reduces security — opens a potential data-exfiltration + vector through the trustd service," so the user decides whether + to keep it. macOS-only, and a no-op outside the sandbox, e.g. + in CI. 5. **comdev MCP checkouts (`ponymail`, `apache-projects`).** These ASF MCP servers are installed from a local `apache/comdev` checkout and are **tracked at `main`, not pinned** — unlike the diff --git a/tools/sandbox-lint/tests/test_doc_domain_consistency.py b/tools/sandbox-lint/tests/test_doc_domain_consistency.py new file mode 100644 index 00000000..afe3c5b9 --- /dev/null +++ b/tools/sandbox-lint/tests/test_doc_domain_consistency.py @@ -0,0 +1,75 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""The update skill's `allowedDomains` prose must match the dogfooded settings. + +`setup-isolated-setup-update` tells the agent which sandbox domains are a +current default and which were deliberately dropped. When +`.claude/settings.json` moves and that prose does not, the skill reports the +absence of a removed host as drift and walks adopters into re-adding dead +allowlist entries. These tests read both sides and fail on that divergence. +""" + +from __future__ import annotations + +import json +import re +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[3] +SETTINGS = REPO_ROOT / ".claude" / "settings.json" +SKILL = REPO_ROOT / "plugins" / "magpie-setup" / "skills" / "isolated-setup-update" / "SKILL.md" + +# The two prose anchors the assertions parse. Rewording the bullet is fine; +# keeping these phrases is what makes the claim machine-checkable. +KEPT_RE = re.compile(r"default allows (.+?), the\s+only hosts", re.DOTALL) +DROPPED_RE = re.compile(r"once sat beside them\s*\((.+?)\) were dropped", re.DOTALL) + + +def _backticked(blob: str) -> set[str]: + return set(re.findall(r"`([^`]+)`", blob)) + + +def _settings_domains() -> set[str]: + settings = json.loads(SETTINGS.read_text()) + return set(settings["sandbox"]["network"]["allowedDomains"]) + + +def _skill_text() -> str: + return SKILL.read_text() + + +def test_kept_domains_are_actually_allowed() -> None: + match = KEPT_RE.search(_skill_text()) + assert match, "the skill no longer states which domains the default allows" + kept = _backticked(match.group(1)) + assert kept, "no domains parsed out of the 'default allows' sentence" + assert kept <= _settings_domains(), ( + "the update skill names domains as the dogfooded default that " + f"{SETTINGS} does not allow: {sorted(kept - _settings_domains())}" + ) + + +def test_dropped_domains_are_not_allowed_again() -> None: + match = DROPPED_RE.search(_skill_text()) + assert match, "the skill no longer lists the dropped link-target hosts" + dropped = _backticked(match.group(1)) + assert dropped, "no domains parsed out of the dropped-hosts list" + readded = dropped & _settings_domains() + assert not readded, ( + f"the update skill says these hosts were dropped, but {SETTINGS} allows them again: {sorted(readded)}" + )