mirror of
https://github.com/MadsLorentzen/ai-job-search.git
synced 2026-11-02 13:12:50 +00:00
fix(guards): refuse a .npmrc beside a portal manifest, like bunfig.toml
Bun reads .npmrc too, so a registry line beside a borrowed portal's manifest fetches every dependency from a server the portal's author picked - the same door bunfig.toml opens, which the installer and guard check 3 already refuse. Both now reject either file. The #718 integration test, which pointed Bun at its offline registry through a .npmrc, passes the registry through NPM_CONFIG_REGISTRY instead; it still proves the hook stays off, because the hooked package only exists on that server. Follows GHSA-wq7j-x5jq-pjrw; this case is ours, not the report's. Guard tests for the workspaces and bunfig.toml refusals, which the previous commit added without them, come along. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
1 parent
0bcfd75e60
commit
83c788d779
9 files changed
+62
-23
No files matched your search
+7
-6
@@ -520,17 +520,18 @@ per-file diff commands.
|
||||
key is added to the allowlist in the same PR, with its own check. Reported privately by
|
||||
@folovco and @Huge (Martin Vlach).
|
||||
|
||||
- **Portal CLI installation never runs a lifecycle script, and refuses `workspaces` and
|
||||
`bunfig.toml`** (#718 by @rudycelekli, `tools/install_portal_clis.py`, `tools/security_guards.py`
|
||||
- **Portal CLI installation never runs a lifecycle script, and refuses `workspaces`,
|
||||
`bunfig.toml` and `.npmrc`** (#718 by @rudycelekli, `tools/install_portal_clis.py`, `tools/security_guards.py`
|
||||
check 3, `tests/test_install_portal_hooks.py`, `tests/test_install_portal_clis.py`) - the
|
||||
installer refused a manifest's own lifecycle scripts and `trustedDependencies`, then ran plain
|
||||
`bun install`, which on Bun 1.3.x still runs the hooks of default-trusted dependencies (esbuild's
|
||||
`postinstall` ran in the reproduction) and of workspace packages: a `workspaces` entry pulled in
|
||||
sub-packages whose scripts the check never read, and a `bunfig.toml` beside the manifest
|
||||
reconfigured the install itself (registry, scopes, trusted packages). Every install now passes
|
||||
sub-packages whose scripts the check never read, and a `bunfig.toml` or `.npmrc` beside the
|
||||
manifest reconfigured the install itself (registry, scopes, trusted packages), so every
|
||||
dependency could come from a server the portal's author picked. Every install now passes
|
||||
`--ignore-scripts` (verified against an offline registry serving a hooked package), the
|
||||
installer skips a manifest that declares `workspaces` or has a `bunfig.toml` beside it, and the
|
||||
in-tree guard forbids both. Reported privately by @folovco and @Huge (Martin Vlach).
|
||||
installer skips a manifest that declares `workspaces` or has a `bunfig.toml` or `.npmrc` beside
|
||||
it, and the in-tree guard forbids all three. The `.npmrc` case is ours, not the report's. Reported privately by @folovco and @Huge (Martin Vlach).
|
||||
|
||||
|
||||
- **The ask list now covers the files the allow list runs, and the guard reads load-time commands**
|
||||
|
||||
@@ -120,7 +120,7 @@ python3 tools/install_portal_clis.py # Windows: python tools\install_portal_cl
|
||||
<details>
|
||||
<summary>What the installer does</summary>
|
||||
|
||||
It runs `bun install` in every portal CLI under `.agents/skills/*/cli/` - discovered, not listed, so a portal you add with `/add-portal` or copy from another fork is included on a fresh clone. Every install runs with `--ignore-scripts`, and a CLI whose `package.json` declares an install-time lifecycle script, `trustedDependencies` or `workspaces`, or that has a `bunfig.toml` beside it, is skipped and reported rather than run. `--dry-run` shows what it would install.
|
||||
It runs `bun install` in every portal CLI under `.agents/skills/*/cli/` - discovered, not listed, so a portal you add with `/add-portal` or copy from another fork is included on a fresh clone. Every install runs with `--ignore-scripts`, and a CLI whose `package.json` declares an install-time lifecycle script, `trustedDependencies` or `workspaces`, or that has a `bunfig.toml` or `.npmrc` beside it, is skipped and reported rather than run. `--dry-run` shows what it would install.
|
||||
|
||||
For `linkedin-search` and `freehire-search` the install is optional: both have zero runtime dependencies and run with plain `bun`; `bun install` only pulls TypeScript dev types.
|
||||
|
||||
|
||||
+1
-1
@@ -31,7 +31,7 @@ Every grant the template ships, where it lives, how long it lasts, and what chec
|
||||
| Hooks (`.claude/settings.json` `hooks`, skill and command frontmatter `hooks`, !`command` lines) | None. The template ships no hooks: a hook runs with no prompt the moment its event fires, which is the vector the Shai-Hulud worm used in August 2026. A !`command` line in a skill or command runs when it loads, the same way. | - | Any of them fails CI |
|
||||
| Every other `settings.json` key | None. `permissions` (with `allow`, `ask`, `deny`) and `hooks` are the whole file: `env` would reach every pre-approved `python tools/*.py` run, `apiKeyHelper` and a `statusLine` command run a program with no prompt, and `permissions.defaultMode` or `additionalDirectories` would widen what the lists above scope. | - | Any other top-level or `permissions` key fails CI (`ALLOWED_SETTINGS_KEYS`, `ALLOWED_PERMISSIONS_KEYS`) |
|
||||
| Compiling (`lualatex`, `xelatex`, a custom template's command) | Not pre-approved: a `lualatex:*` grant would also approve `-shell-escape`. In default mode each compile prompts. Answering "don't ask again" would persist exactly that `lualatex` rule to your `settings.local.json`, so answer per call. | - | - |
|
||||
| Portal CLI manifests (`.agents/**/package.json`) | No lifecycle scripts (`preinstall`, `postinstall`, ...), no `trustedDependencies`, no `workspaces` (whose packages bring their own scripts) and no `bunfig.toml` beside the manifest (which reconfigures the install), so `bun install` runs no code; the installer passes `--ignore-scripts` as well | - | Fails CI; `tools/install_portal_clis.py` also skips a manifest that carries one |
|
||||
| Portal CLI manifests (`.agents/**/package.json`) | No lifecycle scripts (`preinstall`, `postinstall`, ...), no `trustedDependencies`, no `workspaces` (whose packages bring their own scripts) and no `bunfig.toml` or `.npmrc` beside the manifest (either can point every dependency at another registry), so `bun install` runs no code from a source the template did not review; the installer passes `--ignore-scripts` as well | - | Fails CI; `tools/install_portal_clis.py` also skips a manifest that carries one |
|
||||
| Personal-data ignore rules (`.gitignore`) | Every rule in `REQUIRED_IGNORE_RULES` present, read as git reads them (leading whitespace is part of the pattern); no negation outside `ALLOWED_IGNORE_NEGATIONS`, in the root file or in any subdirectory `.gitignore`, which outranks it | - | Fails CI |
|
||||
| The browser-header retry (`tools/fetch_page.py`) | A GET with a fixed argv, http(s) only, 45 s / 5 redirects / 10 MB, `robots.txt` enforced before each redirect target with no bypass flag, output refused inside the repo | - | Pinned by `tests/test_fetch_page.py`; the grant names the script, not `curl` |
|
||||
|
||||
|
||||
@@ -196,7 +196,7 @@ Run these from the repository root.
|
||||
python3 tools/install_portal_clis.py # Windows: python tools\install_portal_clis.py
|
||||
```
|
||||
|
||||
It runs `bun install` in every portal CLI under `.agents/skills/*/cli/` - discovered, not listed, so a portal you add with `/add-portal` or copy from another fork is included on a fresh clone. Every install runs with `--ignore-scripts`, and a CLI whose `package.json` declares an install-time lifecycle script, `trustedDependencies` or `workspaces`, or that has a `bunfig.toml` beside it, is skipped and reported rather than run. `--dry-run` shows what it would install.
|
||||
It runs `bun install` in every portal CLI under `.agents/skills/*/cli/` - discovered, not listed, so a portal you add with `/add-portal` or copy from another fork is included on a fresh clone. Every install runs with `--ignore-scripts`, and a CLI whose `package.json` declares an install-time lifecycle script, `trustedDependencies` or `workspaces`, or that has a `bunfig.toml` or `.npmrc` beside it, is skipped and reported rather than run. `--dry-run` shows what it would install.
|
||||
|
||||
For `linkedin-search` and `freehire-search` the install is optional: both have zero runtime dependencies and run with plain `bun`; `bun install` only pulls TypeScript dev types.
|
||||
|
||||
|
||||
@@ -83,6 +83,13 @@ class InstallPortalClis(unittest.TestCase):
|
||||
(cli / "bunfig.toml").write_text('[install]\nregistry = "https://registry.example"\n', encoding="utf-8")
|
||||
self.assertIn("bunfig.toml", unsafe_reason(cli))
|
||||
|
||||
def test_npmrc_is_unsafe(self):
|
||||
# Bun reads .npmrc too: a registry line beside the manifest fetches
|
||||
# every dependency from a server the portal's author picked.
|
||||
cli = self.add_cli("x")
|
||||
(cli / ".npmrc").write_text("registry=https://registry.example/\n", encoding="utf-8")
|
||||
self.assertIn(".npmrc", unsafe_reason(cli))
|
||||
|
||||
def test_bun_install_never_runs_lifecycle_scripts(self):
|
||||
# #718: --ignore-scripts on every install, so a dependency's hook stays
|
||||
# off whatever the Bun release's default trust list says.
|
||||
|
||||
@@ -72,10 +72,12 @@ class DependencyHooks(unittest.TestCase):
|
||||
(cli / "package.json").write_text(json.dumps({
|
||||
"name": "example-search", "private": True, "dependencies": {"esbuild": "0.25.0"},
|
||||
}), encoding="utf-8")
|
||||
(cli / ".npmrc").write_text(f"registry=http://127.0.0.1:{server.server_port}/\n", encoding="utf-8")
|
||||
# The registry goes in through the environment: a .npmrc beside
|
||||
# the manifest is one of the things the installer now refuses.
|
||||
env = {"PATH": str(Path(BUN).absolute().parent) + os.pathsep + os.defpath,
|
||||
"HOME": str(root), "TMPDIR": str(root), "TEMP": str(root), "TMP": str(root),
|
||||
"BUN_INSTALL_CACHE_DIR": str(root / "cache")}
|
||||
"BUN_INSTALL_CACHE_DIR": str(root / "cache"),
|
||||
"NPM_CONFIG_REGISTRY": f"http://127.0.0.1:{server.server_port}/"}
|
||||
if "SYSTEMROOT" in os.environ:
|
||||
env["SYSTEMROOT"] = os.environ["SYSTEMROOT"]
|
||||
proc = subprocess.run([sys.executable, str(tools / "install_portal_clis.py")],
|
||||
|
||||
@@ -874,6 +874,30 @@ class ManifestGuardTests(GuardRepoFixture):
|
||||
self.assertEqual(result.returncode, 1)
|
||||
self.assertIn("trustedDependencies", result.stdout)
|
||||
|
||||
# GHSA-wq7j-x5jq-pjrw: the other doors into `bun install`. Workspace
|
||||
# packages carry scripts this check never reads; a config file beside the
|
||||
# manifest (bunfig.toml, .npmrc) can point every dependency at another
|
||||
# registry. The installer refuses the same things.
|
||||
def test_workspaces_fails(self):
|
||||
self.write_manifest({"name": "example-cli", "workspaces": ["packages/*"]})
|
||||
result = run_guards(self.root)
|
||||
self.assertEqual(result.returncode, 1)
|
||||
self.assertIn("workspaces", result.stdout)
|
||||
|
||||
def test_install_config_file_beside_manifest_fails(self):
|
||||
for name, body in (("bunfig.toml", '[install]\nregistry = "https://registry.example/"\n'),
|
||||
(".npmrc", "registry=https://registry.example/\n")):
|
||||
with self.subTest(file=name):
|
||||
config = self.manifest.parent / name
|
||||
config.write_text(body, encoding="utf-8")
|
||||
try:
|
||||
result = run_guards(self.root)
|
||||
finally:
|
||||
config.unlink()
|
||||
self.assertEqual(result.returncode, 1, result.stdout)
|
||||
self.assertIn(name, result.stdout)
|
||||
self.assertIn("example-search/cli/" + name, result.stdout.replace("\\", "/"))
|
||||
|
||||
def test_malformed_manifest_shape_fails_cleanly(self):
|
||||
for data, message in [
|
||||
([], "top-level JSON value must be an object"),
|
||||
|
||||
@@ -14,7 +14,7 @@ README previously carried a PowerShell and a Bash loop, each with its own
|
||||
hand-copied list of portal names.
|
||||
|
||||
A manifest that declares an npm/bun lifecycle script, trustedDependencies or
|
||||
workspaces, or that has a bunfig.toml beside it, is skipped and reported
|
||||
workspaces, or that has a bunfig.toml or .npmrc beside it, is skipped and reported
|
||||
instead of installed: those are the doors that run code or reconfigure the
|
||||
install during `bun install`, and tools/security_guards.py forbids them
|
||||
in-tree. A borrowed portal that carries one gets read by a human before it
|
||||
@@ -38,7 +38,7 @@ from pathlib import Path
|
||||
ROOT = Path(__file__).resolve().parent.parent
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parent))
|
||||
|
||||
from security_guards import FORBIDDEN_SCRIPTS # noqa: E402
|
||||
from security_guards import FORBIDDEN_SCRIPTS, INSTALL_CONFIG_FILES # noqa: E402
|
||||
|
||||
|
||||
def discover(root: Path = ROOT) -> list[Path]:
|
||||
@@ -64,9 +64,10 @@ def unsafe_reason(cli_dir: Path) -> str | None:
|
||||
# Workspace packages carry their own lifecycle scripts, which the check
|
||||
# above never reads; a workspace postinstall ran under Bun 1.3.10.
|
||||
return "declares workspaces"
|
||||
if (cli_dir / "bunfig.toml").exists():
|
||||
for name in INSTALL_CONFIG_FILES:
|
||||
if (cli_dir / name).exists():
|
||||
# Reconfigures the install itself: registry, scopes, trusted packages.
|
||||
return "carries a bunfig.toml"
|
||||
return f"carries a {name}"
|
||||
return None
|
||||
|
||||
|
||||
|
||||
@@ -27,8 +27,8 @@ Checks:
|
||||
a subdirectory, which outranks the root file, is checked for negations.
|
||||
3. .agents/**/package.json — no npm/bun lifecycle scripts (preinstall,
|
||||
install, postinstall, prepare, prepack), no trustedDependencies, no
|
||||
workspaces, and no bunfig.toml beside the manifest. Catches code
|
||||
execution smuggled into `bun install`.
|
||||
workspaces, and no bunfig.toml or .npmrc beside the manifest. Catches
|
||||
code execution smuggled into `bun install`, and a registry swap.
|
||||
4. Skill and command frontmatter — every `allowed-tools` entry in
|
||||
.claude/skills/*/SKILL.md, .agents/skills/*/SKILL.md and
|
||||
.claude/commands/*.md must be in ALLOWED_SKILL_TOOLS (a portal skill may
|
||||
@@ -254,10 +254,13 @@ FORBIDDEN_SCRIPTS = {"preinstall", "install", "postinstall", "prepare", "prepack
|
||||
# Manifest keys that pull code into `bun install` by another door: `workspaces`
|
||||
# installs sub-packages with their own lifecycle scripts (a workspace
|
||||
# postinstall ran under Bun 1.3.10, scripts the top-level check never saw), and
|
||||
# `trustedDependencies` re-enables dependency scripts. A bunfig.toml beside the
|
||||
# manifest reconfigures the install itself (registry, scopes, cache) and is
|
||||
# refused for the same reason. tools/install_portal_clis.py skips all of these.
|
||||
# `trustedDependencies` re-enables dependency scripts. A bunfig.toml or .npmrc
|
||||
# beside the manifest reconfigures the install itself (registry, scopes,
|
||||
# cache), so every dependency can come from a server the portal's author
|
||||
# picked; both are refused for the same reason. tools/install_portal_clis.py
|
||||
# skips all of these.
|
||||
FORBIDDEN_MANIFEST_KEYS = {"trustedDependencies", "workspaces"}
|
||||
INSTALL_CONFIG_FILES = ("bunfig.toml", ".npmrc")
|
||||
|
||||
# The exact `allowed-tools` entries a skill or command frontmatter may carry.
|
||||
# Claude Code pre-approves these for the turn that invokes the skill, with no
|
||||
@@ -666,10 +669,11 @@ def check_package_manifests() -> None:
|
||||
f"{relpath}: workspaces is forbidden - each workspace package brings its own "
|
||||
"lifecycle scripts, which this check never reads, into `bun install`."
|
||||
)
|
||||
bunfig = manifest.parent / "bunfig.toml"
|
||||
if bunfig.exists():
|
||||
for name in INSTALL_CONFIG_FILES:
|
||||
config = manifest.parent / name
|
||||
if config.exists():
|
||||
errors.append(
|
||||
f"{bunfig.relative_to(ROOT)}: a bunfig.toml beside a portal manifest is forbidden - "
|
||||
f"{config.relative_to(ROOT)}: a {name} beside a portal manifest is forbidden - "
|
||||
"it reconfigures `bun install` itself (registry, scopes, trusted packages) for "
|
||||
"every fork user."
|
||||
)
|
||||
|
||||
Reference in new issue
Block a user