diff --git a/.github/workflows/npm-nested-metadata-validation-quality-ci.yml b/.github/workflows/npm-nested-metadata-validation-quality-ci.yml new file mode 100644 index 000000000..4f9d0a5bd --- /dev/null +++ b/.github/workflows/npm-nested-metadata-validation-quality-ci.yml @@ -0,0 +1,136 @@ +name: npm Nested Metadata Validation Quality CI + +on: + pull_request: + branches: [main] + paths: + - ".github/workflows/npm-nested-metadata-validation-quality-ci.yml" + - "scripts/ci/materialize_base_javascript_packages.py" + - "tests/conftest.py" + - "tests/test_javascript_materializer_creation_cleanup_security.py" + - "tests/test_javascript_materializer_descriptor_ancestry.py" + - "tests/test_javascript_materializer_existing_directory_coverage.py" + - "tests/test_javascript_materializer_output_edge_coverage.py" + - "tests/test_javascript_materializer_output_security.py" + - "tests/test_materialize_base_javascript_packages.py" + - "tests/test_npm_nested_metadata_lock_validation.py" + - "docs/doctoring/npm-nested-metadata-canonical-pins.md" + - "requirements-opencode-review-ci-hashes.txt" + - "CHANGELOG.md" + push: + branches: [main] + paths: + - ".github/workflows/npm-nested-metadata-validation-quality-ci.yml" + - "scripts/ci/materialize_base_javascript_packages.py" + - "tests/conftest.py" + - "tests/test_javascript_materializer_creation_cleanup_security.py" + - "tests/test_javascript_materializer_descriptor_ancestry.py" + - "tests/test_javascript_materializer_existing_directory_coverage.py" + - "tests/test_javascript_materializer_output_edge_coverage.py" + - "tests/test_javascript_materializer_output_security.py" + - "tests/test_materialize_base_javascript_packages.py" + - "tests/test_npm_nested_metadata_lock_validation.py" + - "docs/doctoring/npm-nested-metadata-canonical-pins.md" + - "requirements-opencode-review-ci-hashes.txt" + - "CHANGELOG.md" + +concurrency: + group: npm-nested-metadata-quality-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +env: + FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true + +jobs: + python-310-compatibility: + name: Python 3.10 compatibility + runs-on: ubuntu-24.04 + timeout-minutes: 10 + steps: + - name: Harden runner + uses: step-security/harden-runner@bf7454d06d71f1098171f2acdf0cd4708d7b5920 # v2.20.0 + with: + egress-policy: audit + - name: Checkout exact source revision + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + ref: ${{ github.event.pull_request.head.sha || github.sha }} + persist-credentials: false + - name: Set up Python 3.10 + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: "3.10" + - name: Compile implementation and contracts + run: | + python -m compileall -q \ + scripts/ci/materialize_base_javascript_packages.py \ + tests/conftest.py \ + tests/test_javascript_materializer_descriptor_ancestry.py \ + tests/test_javascript_materializer_existing_directory_coverage.py \ + tests/test_javascript_materializer_output_edge_coverage.py \ + tests/test_javascript_materializer_output_security.py \ + tests/test_materialize_base_javascript_packages.py \ + tests/test_npm_nested_metadata_lock_validation.py + + python-314-quality: + name: Python 3.14 complete quality + runs-on: ubuntu-24.04 + timeout-minutes: 30 + steps: + - name: Harden runner + uses: step-security/harden-runner@bf7454d06d71f1098171f2acdf0cd4708d7b5920 # v2.20.0 + with: + egress-policy: audit + - name: Checkout exact source revision + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + ref: ${{ github.event.pull_request.head.sha || github.sha }} + persist-credentials: false + - name: Set up Python 3.14 + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: "3.14" + cache: pip + cache-dependency-path: requirements-opencode-review-ci-hashes.txt + - name: Install exact hash-locked quality tooling + run: >- + python -m pip install --disable-pip-version-check --require-hashes + -r requirements-opencode-review-ci-hashes.txt + - name: Run focused tests with complete production branch coverage + run: | + python -m coverage erase + python -m coverage run --branch -m pytest -q \ + tests/test_javascript_materializer_creation_cleanup_security.py \ + tests/test_javascript_materializer_descriptor_ancestry.py \ + tests/test_javascript_materializer_existing_directory_coverage.py \ + tests/test_javascript_materializer_output_edge_coverage.py \ + tests/test_javascript_materializer_output_security.py \ + tests/test_materialize_base_javascript_packages.py \ + tests/test_npm_nested_metadata_lock_validation.py + python -m coverage report \ + --include=scripts/ci/materialize_base_javascript_packages.py \ + --show-missing \ + --fail-under=100 + - name: Enforce complete production docstrings and compilation + run: | + python -m interrogate --fail-under 100 scripts/ci/materialize_base_javascript_packages.py + python -m compileall -q \ + scripts/ci/materialize_base_javascript_packages.py \ + tests/conftest.py \ + tests/test_javascript_materializer_creation_cleanup_security.py \ + tests/test_javascript_materializer_descriptor_ancestry.py \ + tests/test_javascript_materializer_existing_directory_coverage.py \ + tests/test_javascript_materializer_output_edge_coverage.py \ + tests/test_javascript_materializer_output_security.py \ + tests/test_materialize_base_javascript_packages.py \ + tests/test_npm_nested_metadata_lock_validation.py + - name: Run complete central regression suite + run: | + python -m coverage erase + python -m coverage run -m pytest tests -q + python -m coverage report --show-missing --fail-under=100 + - name: Verify clean patches + run: git diff --check diff --git a/AGENTS.md b/AGENTS.md index bd6a96a11..df124bb22 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -7,3 +7,4 @@ Materialize accepts only exact SHA-256 pins or a bounded relative `-r` include ( Conflict-scope roots fail closed when the immediate parent directory is a symbolic link. OriginWeave hourly NVIDIA NIM repair is a thin caller at minute 10. See [`docs/doctoring/originweave-hourly-review-caller.md`](docs/doctoring/originweave-hourly-review-caller.md). nonnest2 hourly NVIDIA NIM repair is a thin caller at minute 16. See [`docs/doctoring/nonnest2-hourly-review-caller.md`](docs/doctoring/nonnest2-hourly-review-caller.md). +Npm nested metadata pin rules are recorded in [`docs/doctoring/npm-nested-metadata-canonical-pins.md`](docs/doctoring/npm-nested-metadata-canonical-pins.md). diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 7d2bfb4a4..511babadf 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -70,6 +70,30 @@ Product callers stagger Clearfolio at minute 23, DiskSage at minute 37, and fast-mlsirm at minute 49. Each caller is read-only, dispatches at most one repair, and delegates all privileged logic to the same sealed scheduler. +## Nested npm metadata pins + +```mermaid +flowchart TD + Entry["packages map entry"] + Link{"workspace link?"} + Fields{"resolved and integrity?"} + Root{"canonical root?"} + Accept["Accept after HTTPS SHA-512 pin"] + Reject["Fail closed"] + + Entry --> Link + Link -->|"yes"| Accept + Link -->|"no"| Fields + Fields -->|"both"| Accept + Fields -->|"one"| Reject + Fields -->|"neither"| Root + Root -->|"yes"| Reject + Root -->|"no"| Accept +``` + +An explicit port, userinfo, query, or fragment is not the default npm +registry origin. Publication uses no-follow, descriptor-relative opens. + ## Control-plane data flow ```mermaid @@ -125,4 +149,6 @@ trusted `uv` exporter is downloaded from the literal GitHub Releases URL for - [`docs/doctoring/hourly-nvidia-nim-autofix.md`](docs/doctoring/hourly-nvidia-nim-autofix.md) — current increment's repair-worker decision and APA 7th citations. - [`docs/doctoring/fast-mlsirm-hourly-review-caller.md`](docs/doctoring/fast-mlsirm-hourly-review-caller.md) - — product-specific psychometric repair heartbeat and scientific gates. \ No newline at end of file + — product-specific psychometric repair heartbeat and scientific gates. +- [`docs/doctoring/npm-nested-metadata-canonical-pins.md`](docs/doctoring/npm-nested-metadata-canonical-pins.md) + — current increment's lockfile decision and APA 7th citations. diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d2f9f24d..0ee0c74d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,10 @@ Semantic Versioning where the repository publishes a release. - Compared the trusted `uv` executable's post-install `--version` output against the real GitHub Releases build's full string, `uv 0.12.1 (x86_64-unknown-linux-gnu)`, instead of the bare `uv 0.12.1` the prior check required; the genuine release binary always prints the target triple, so every installation was failing the pin check immediately after the archive download itself was fixed (ContextualWisdomLab/.github#1109). - Excluded relative `-r` and `--requirement` referrers from generated flat base-lock publication while retaining bounded include syntax diagnostics and discovering independently complete direct `.txt` children of `requirements` directories. - Refused a conflict-scope repository root whose immediate parent is a symbolic link, so a swapped parent cannot redirect the canonical worktree after the last-component check (CWE-367). +- Accepted npm v2/v3 metadata-only nested workspace and peer locations only when one exact scoped or unscoped canonical root package carries the same version, HTTPS npm-registry tarball, and canonical SHA-512 integrity, while continuing to reject malformed identities, partial pins, metadata-only roots, alternate origins, and version drift. The decision record now cites RFC 3986 so an explicit port, userinfo, query, or fragment cannot masquerade as the default npm registry origin. +- Made Python lock evidence publication use the same descriptor-relative, no-follow, exclusive-write and inode-binding boundary as JavaScript lock evidence, closing TOCTOU paths that could redirect generated requirements or manifests during materialization. +- Recorded the org control-plane architecture, including nested npm metadata pins, so agents reconstruct the lockfile trust boundary from the repo instead of private memory. +- Made JavaScript lock evidence publication fail before mutation without descriptor/no-follow capabilities, create every output component from pinned directory descriptors, and roll back only inode-matched entries from a freshly owned project subtree after late write failure. - Bounded the Strix quality self-test's deterministic timeout fixtures to 3-second process and 5-second fake-sleep budgets so exact-head policy evidence completes inside the existing job limit without changing production Strix scanner timeouts, providers, credentials, or review semantics. - Allowed commas and ASCII parentheses in the bounded Strix changed-file path policy so legal tracked Packrat fixtures can receive exact-head security analysis, while rejecting raw `..` components before normalization and keeping controls, backslashes, whitespace ambiguity, and shell punctuation fail-closed. - Bound each review-agent invocation key to the wrapper's complete canonical payload, including the base branch and requesting actor; altered fields with a valid-format key now fail before durable-leader election or forwarding, and wrapper write permission is job-scoped. diff --git a/CLAUDE.md b/CLAUDE.md index 6ec3d494c..33024f36a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -69,7 +69,8 @@ Details: `docs/pr-review-and-merge-procedure.md` and `PR_GOVERNANCE_AUDIT.md`. - `docs/` — master context, Project protocol, `org-required-workflow-rollout.md`, `scorecard-governance.md`, SBOM inventory. Doctoring records live under `docs/doctoring/`. [`ARCHITECTURE.md`](ARCHITECTURE.md) is the control-plane - diagram for review, hourly NVIDIA NIM repair, and merge trust boundaries. + diagram for review, hourly NVIDIA NIM repair, nested npm metadata pins, and + merge trust boundaries. - `.jules/` — recorded performance (`bolt.md`) and security (`sentinel.md`) learnings from past work on `scripts/ci/`; worth scanning before optimizing or hardening those scripts. diff --git a/docs/doctoring/npm-nested-metadata-canonical-pins.md b/docs/doctoring/npm-nested-metadata-canonical-pins.md new file mode 100644 index 000000000..755f6e85d --- /dev/null +++ b/docs/doctoring/npm-nested-metadata-canonical-pins.md @@ -0,0 +1,101 @@ +# npm nested metadata canonical pins + +## Decision + +Changed npm lockfiles remain untrusted pull-request inputs. The central JavaScript dependency materializer accepts npm lockfile versions 2 and 3 only after validating the complete `packages` map. Every non-link package location under a `node_modules` segment must declare a nonempty exact `version`. + +npm can serialize a nested workspace or peer location with version and classification metadata while the canonical root location carries the registry tarball and integrity fields. The validator therefore distinguishes two safe forms: + +1. **Complete pin** — the location declares both `resolved` and `integrity`. The URL must be an HTTPS tarball on `registry.npmjs.org` with no user information, explicit port, query, or fragment, and the integrity value must be one canonical SHA-512 SRI value. +2. **Metadata-only nested location** — the location declares neither field. It is accepted only when `node_modules/` contains one complete pin for the same scoped or unscoped package identity and the exact same version. + +A metadata-only canonical root entry is forbidden. A nested location that declares only one of `resolved` or `integrity` is also forbidden. Independently complete nested pins remain valid and may carry a different version because their bytes and integrity are self-contained. + +## Package identity + +Identity is derived from the path segments after the final `node_modules` component: + +- unscoped: exactly one segment, such as `react`; +- scoped: exactly two segments, such as `@types/react-dom`. + +Incomplete scopes, additional identity segments, absolute paths, backslashes, and parent traversal fail closed. Workspace links retain their separate bounded relative-link validation and never inherit registry metadata. + +```mermaid +flowchart TD + A[npm packages map entry] --> B{link is true?} + B -->|yes| C[Validate bounded relative workspace target] + B -->|no| D[Derive exact package identity and require version] + D --> E{resolved and integrity} + E -->|both present| F[Validate exact npm registry tarball and SHA-512 SRI] + E -->|one present| G[Reject partial pin] + E -->|both absent| H{canonical root?} + H -->|yes| I[Reject metadata-only root] + H -->|no| J[Require same identity and version at complete root pin] +``` + +## Security and compatibility boundary + +The policy does not repair, synthesize, or mutate lockfile metadata. It consumes the validated lock unchanged. It preserves the existing lockfile version, path, link, URL, origin, tarball suffix, and SHA-512 controls while admitting npm's location-keyed metadata representation. + +RFC 3986 treats userinfo, port, query, and fragment as distinct URI +components that change origin identity (Berners-Lee et al., 2005). An +explicit `:443` is therefore not the same pin as the default HTTPS origin +`registry.npmjs.org`. The validator rejects every explicit port, not only +non-default ones. + +The canonical root pin is a provenance anchor for metadata-only locations, not a claim that all nested locations share one physical installation. A complete nested record is validated independently and does not depend on the root. Missing roots, version drift, malformed identity, partial fields, alternate registries, malformed URLs, and invalid integrity remain blocking. + +### Filesystem publication boundary + +Materialized JavaScript and Python evidence is published only when the runtime supports descriptor-relative directory operations, descriptor-backed enumeration, `O_DIRECTORY`, `O_NOFOLLOW`, and no-follow `stat`. The capability gate runs before any output path is created. Missing output components are then created and opened one component at a time from a held filesystem-root descriptor; each name is inspected without following links, opened relative to its pinned parent, and matched to the observed device/inode identity. The final absolute pathname must still identify the pinned output directory before any project file is written. + +Generated files use exclusive, no-follow descriptor-relative creation, forward-progress-checked writes, file and directory synchronization, and post-write identity and link-count validation. A project directory is fresh and owned exclusively by one attempt. If a later write fails, cleanup walks only that held project descriptor, removes only inode-matched regular files and directories in reverse publication order, and never follows links. A raced, replaced, symlink, or special entry is retained for forensic inspection; cleanup never masks the original fail-closed error or removes pre-existing operator entries outside the owned project directory. + +## Verification + +The permanent regression suite includes: + +- the BandScope `apps/desktop/node_modules/@types/react-dom` peer-location shape; +- unscoped metadata-only locations; +- independently pinned nested versions; +- missing canonical pins; +- canonical-version mismatch; +- metadata-only canonical roots; +- partial `resolved` or `integrity` declarations; +- malformed scoped identities; +- nonempty-version enforcement; +- alternate origins and invalid SHA-512 SRI values; and +- all pre-existing npm path, link, lockfile, URL, and integrity cases; +- missing descriptor/no-follow capabilities before mutation; +- missing-ancestor and intermediate-ancestor replacement races; +- nested-directory and generated-file identity replacement; and +- late-write rollback that preserves pre-existing operator data. + +The dedicated quality workflow runs Python 3.10 compilation, Python 3.14 focused tests with 100% production statement and branch coverage, 100% production docstrings, the complete central test suite, and a clean-patch check. The Python materializer regression tests additionally cover capability refusal, ancestor and output rebinding, exclusive creation, non-regular targets, partial-write cleanup, and final identity validation. + +## Incident recovery and rollback + +1. Preserve the exact pull-request head SHA, lockfile blob SHA, validation error, and quality-run ID. +2. Determine whether the changed lock is malformed or whether npm produced a supported metadata-only nested location. +3. Never add missing tarball or integrity values by hand. Regenerate the lock with the repository's pinned npm version when the lock is invalid. +4. Preserve any raced or unexpected filesystem entry for forensic inspection. Do not replace descriptor-relative cleanup with recursive pathname deletion. +5. Roll back only by restoring the prior fail-closed validator or another reviewed implementation that keeps the same identity, version, origin, integrity, no-follow publication, and owned-object cleanup controls. +6. Rerun the complete exact-head quality, security, and supply-chain matrix after any repair. + +## References + +Berners-Lee, T., Fielding, R., & Masinter, L. (2005). *Uniform Resource +Identifier (URI): Generic syntax* (RFC 3986). Internet Engineering Task +Force. https://doi.org/10.17487/RFC3986 + +npm, Inc. (2026). *package-lock.json*. npm Docs. https://docs.npmjs.com/cli/v11/configuring-npm/package-lock-json + +npm, Inc. (2026). *npm install*. npm Docs. https://docs.npmjs.com/cli/v11/commands/npm-install + +World Wide Web Consortium. (2016). *Subresource Integrity*. https://www.w3.org/TR/SRI/ + +Institute of Electrical and Electronics Engineers, & The Open Group. (2024). *The Open Group Base Specifications Issue 8: IEEE Std 1003.1-2024*. https://pubs.opengroup.org/onlinepubs/9799919799/ + +MITRE Corporation. (2026). *CWE-59: Improper link resolution before file access ('link following')* (Version 4.20). https://cwe.mitre.org/data/definitions/59.html + +MITRE Corporation. (2026). *CWE-367: Time-of-check time-of-use (TOCTOU) race condition* (Version 4.20). https://cwe.mitre.org/data/definitions/367.html diff --git a/docs/superpowers/patches/2026-08-07-npm-descriptor-safe-output.part00 b/docs/superpowers/patches/2026-08-07-npm-descriptor-safe-output.part00 new file mode 100644 index 000000000..ad46758fd --- /dev/null +++ b/docs/superpowers/patches/2026-08-07-npm-descriptor-safe-output.part00 @@ -0,0 +1,197 @@ +diff --git a/docs/doctoring/npm-materializer-descriptor-safety.md b/docs/doctoring/npm-materializer-descriptor-safety.md +new file mode 100644 +index 0000000..7a839cc +--- /dev/null ++++ b/docs/doctoring/npm-materializer-descriptor-safety.md +@@ -0,0 +1,59 @@ ++# Descriptor-safe trusted JavaScript materialization ++ ++## Decision ++ ++Trusted base and bounded current-head JavaScript inputs are written only through ++held POSIX directory descriptors. Every descendant directory is created with ++`mkdirat` semantics and reopened with `O_DIRECTORY | O_NOFOLLOW`; every file is ++created with `O_CREAT | O_EXCL | O_NOFOLLOW`, written with an explicit ++forward-progress loop, and synchronized together with its parent directory. ++ ++The implementation deliberately fails closed on runtimes without the required ++`dir_fd`, `O_DIRECTORY`, and `O_NOFOLLOW` primitives. It never falls back to ++`Path.mkdir`, `Path.write_bytes`, or `Path.write_text`, because pathname ++re-resolution would recreate the same time-of-check/time-of-use boundary the ++control is intended to remove. ++ ++## Threat model ++ ++A concurrent process under the same runner identity may rename an output root or ++nested project directory and replace the pathname with a symbolic link after a ++validation check. Held directory descriptors remain bound to the opened inode; ++subsequent relative opens either continue inside that inode or fail closed when ++a nested component was replaced. Pre-existing files and descendant directories ++are not adopted or overwritten. ++ ++## Failure and cleanup ++ ++Only the final output root may be created; its parent hierarchy must already ++exist. A zero-progress write is an error. Partially created files are unlinked ++through their parent descriptor. Context-manager failure cleanup removes only ++files and directories owned by the current invocation and removes the output ++root only when that invocation created it. A pre-existing empty root remains ++caller-owned. ++ ++## Verification ++ ++Permanent regressions cover root and nested-directory rename/symlink swaps, ++pre-existing files and directories, parent and absolute path rejection, ++zero-progress writes, cleanup ownership and cleanup refusal, descriptor-close ++behavior, constructor races, unexpected operating-system failures, and missing ++secure primitives. The helper itself is held to 100% statement and branch ++coverage. Production use remains limited to POSIX runners until an equivalent ++descriptor/capability implementation is reviewed for another platform. ++ ++## References ++ ++The Open Group. (2018). *The Open Group Base Specifications Issue 7, 2018 ++edition: `openat()`*. IEEE and The Open Group. ++https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html ++ ++Python Software Foundation. (2026). *`os`—Miscellaneous operating system ++interfaces*. Python 3.14 documentation. ++https://docs.python.org/3/library/os.html ++ ++MITRE. (2025). *CWE-59: Improper link resolution before file access (link ++following)*. https://cwe.mitre.org/data/definitions/59.html ++ ++MITRE. (2025). *CWE-367: Time-of-check time-of-use (TOCTOU) race condition*. ++https://cwe.mitre.org/data/definitions/367.html +diff --git a/scripts/ci/materialize_base_javascript_packages.py b/scripts/ci/materialize_base_javascript_packages.py +index 2bdfa2e..52066b8 100644 +--- a/scripts/ci/materialize_base_javascript_packages.py ++++ b/scripts/ci/materialize_base_javascript_packages.py +@@ -12,26 +12,10 @@ import sys + import urllib.parse + from typing import Any + +- +-SHA_RE = re.compile(r"^[0-9a-fA-F]{40}$") ++from secure_output_tree import SecureOutputTree + + +-def _reject_symlinked_output_components(output_dir: pathlib.Path) -> None: +- """Reject existing symlink components before materialization writes begin.""" +- candidate = output_dir.absolute() +- current = pathlib.Path(candidate.anchor) +- for component in candidate.parts[1:]: +- current /= component +- if current.is_symlink(): +- raise ValueError( +- f"output directory must not be a symlink or contain symlinks: {current}" +- ) +- if not current.exists(): +- break +- if not current.is_dir(): +- raise ValueError( +- f"output directory path component must be a directory: {current}" +- ) ++SHA_RE = re.compile(r"^[0-9a-fA-F]{40}$") + + + def materialize( +@@ -41,10 +25,6 @@ def materialize( + head_sha: str | None = None, + ) -> list[dict[str, str]]: + """Write trusted base and bounded HEAD inputs under Docker-context-safe paths.""" +- _reject_symlinked_output_components(output_dir) +- output_dir.mkdir(parents=True, exist_ok=True) +- _reject_symlinked_output_components(output_dir) +- + manifest: list[dict[str, str]] = [] + projects: list[tuple[str, str, dict[str, bytes], str, str]] = [] + base_npm = base_npm_projects(repo_root, base_sha) +@@ -87,32 +67,33 @@ def materialize( + ) + ) + +- for index, ( +- source_path, +- package_manager, +- base_inputs, +- revision_sha, +- lock_blob, +- ) in enumerate(sorted(projects, key=lambda project: (project[0], project[3]))): +- directory = f"project-{index:03d}" +- project_dir = output_dir / directory +- project_dir.mkdir() +- for relative_path, content in sorted(base_inputs.items()): +- destination = project_dir / relative_path +- destination.parent.mkdir(parents=True, exist_ok=True) +- destination.write_bytes(content) +- manifest.append( +- { +- "directory": directory, +- "lock_blob": lock_blob, +- "package_manager": package_manager, +- "revision_sha": revision_sha, +- "source": source_path, +- } +- ) ++ with SecureOutputTree(output_dir) as output_tree: ++ for index, ( ++ source_path, ++ package_manager, ++ base_inputs, ++ revision_sha, ++ lock_blob, ++ ) in enumerate(sorted(projects, key=lambda project: (project[0], project[3]))): ++ directory = f"project-{index:03d}" ++ output_tree.mkdir(directory) ++ for relative_path, content in sorted(base_inputs.items()): ++ output_tree.write_bytes( ++ pathlib.PurePosixPath(directory) / relative_path, ++ content, ++ ) ++ manifest.append( ++ { ++ "directory": directory, ++ "lock_blob": lock_blob, ++ "package_manager": package_manager, ++ "revision_sha": revision_sha, ++ "source": source_path, ++ } ++ ) + +- (output_dir / "manifest.json").write_text( +- json.dumps(manifest, indent=2, sort_keys=True) + "\n", +- encoding="utf-8", +- ) ++ output_tree.write_text( ++ "manifest.json", ++ json.dumps(manifest, indent=2, sort_keys=True) + "\n", ++ ) + return manifest +diff --git a/scripts/ci/secure_output_tree.py b/scripts/ci/secure_output_tree.py +new file mode 100644 +index 0000000..19f6f75 +--- /dev/null ++++ b/scripts/ci/secure_output_tree.py +@@ -0,0 +1,333 @@ ++#!/usr/bin/env python3 ++"""Descriptor-anchored, no-follow output materialization primitives. ++ ++The helper is intentionally POSIX-only. It keeps directory descriptors open, ++creates every descendant relative to those descriptors, and never falls back to ++pathname-based writes. This prevents a concurrent same-UID process from ++redirecting trusted output through rename-and-symlink races. ++""" ++ ++from __future__ import annotations ++ ++import errno ++import os ++import pathlib ++from collections.abc import Iterable ++from types import TracebackType ++from typing import Self, cast ++ ++ ++_DIRECTORY_FLAGS = os.O_RDONLY | getattr(os, "O_DIRECTORY", 0) | getattr(os, "O_CLOEXEC", 0) ++_NOFOLLOW_DIRECTORY_FLAGS = _DIRECTORY_FLAGS | getattr(os, "O_NOFOLLOW", 0) \ No newline at end of file diff --git a/docs/superpowers/patches/2026-08-07-npm-descriptor-safe-output.part01 b/docs/superpowers/patches/2026-08-07-npm-descriptor-safe-output.part01 new file mode 100644 index 000000000..8de23c787 --- /dev/null +++ b/docs/superpowers/patches/2026-08-07-npm-descriptor-safe-output.part01 @@ -0,0 +1,191 @@ +, 0) ++_FILE_FLAGS = ( ++ os.O_WRONLY ++ | os.O_CREAT ++ | os.O_EXCL ++ | getattr(os, "O_CLOEXEC", 0) ++ | getattr(os, "O_NOFOLLOW", 0) ++) ++ ++ ++def _secure_primitives_available() -> bool: ++ """Return whether this runtime exposes every required secure POSIX primitive.""" ++ required_constants = ("O_DIRECTORY", "O_NOFOLLOW") ++ required_dir_fd = (os.open, os.mkdir, os.unlink, os.rmdir) ++ return ( ++ os.name == "posix" ++ and all(hasattr(os, name) for name in required_constants) ++ and all(function in os.supports_dir_fd for function in required_dir_fd) ++ and hasattr(os, "fsync") ++ and hasattr(os, "write") ++ ) ++ ++ ++def _relative_parts(value: str | pathlib.PurePosixPath) -> tuple[str, ...]: ++ """Return validated lexical POSIX components without normalization.""" ++ raw = str(value) ++ if "\x00" in raw or "\\" in raw: ++ raise ValueError("secure output paths must be NUL-free POSIX paths") ++ if raw.startswith("/"): ++ raise ValueError("secure output paths must be relative") ++ parts = tuple(raw.split("/")) ++ if not parts or parts == ("",): ++ raise ValueError("secure output paths must not be empty") ++ if any(part in ("", ".", "..") for part in parts): ++ raise ValueError("secure output paths must not contain empty, dot, or parent components") ++ return parts ++ ++ ++def _write_all(file_descriptor: int, content: bytes) -> None: ++ """Write every byte or fail if the operating system makes no forward progress.""" ++ remaining = memoryview(content) ++ while remaining: ++ written = os.write(file_descriptor, remaining) ++ if written <= 0: ++ raise OSError("secure output write made no forward progress") ++ remaining = remaining[written:] ++ ++ ++class SecureOutputTree: ++ """Create one isolated output tree through held no-follow descriptors. ++ ++ Existing output roots may be reused only when target descendants do not ++ already exist. Every directory and file below the root is owned by this ++ instance. On failure the helper removes only descendants it created, and it ++ removes the root itself only when this instance created it. ++ """ ++ ++ def __init__(self, output_dir: pathlib.Path) -> None: ++ """Open or create ``output_dir`` without resolving mutable pathnames.""" ++ if not _secure_primitives_available(): ++ raise RuntimeError( ++ "secure output materialization requires POSIX dir_fd, O_DIRECTORY, and O_NOFOLLOW support" ++ ) ++ raw_parts = pathlib.PurePath(os.fspath(output_dir)).parts ++ if ".." in raw_parts: ++ raise ValueError("output directory must not contain parent components") ++ absolute = output_dir.absolute() ++ if absolute == pathlib.Path(absolute.anchor): ++ raise ValueError("output directory must not be the filesystem root") ++ ++ anchor_fd = os.open(absolute.anchor, _DIRECTORY_FLAGS) ++ current_fd = anchor_fd ++ current_owned = False ++ root_parent_fd: int | None = None ++ root_name: str | None = None ++ created_root = False ++ try: ++ components = absolute.parts[1:] ++ for index, component in enumerate(components): ++ final = index == len(components) - 1 ++ if final: ++ root_parent_fd = os.dup(current_fd) ++ root_name = component ++ created = False ++ try: ++ next_fd = os.open(component, _NOFOLLOW_DIRECTORY_FLAGS, dir_fd=current_fd) ++ except FileNotFoundError: ++ if not final: ++ raise ValueError( ++ "output directory parent components must already exist" ++ ) ++ os.mkdir(component, mode=0o700, dir_fd=current_fd) ++ os.fsync(current_fd) ++ created = True ++ try: ++ next_fd = os.open( ++ component, ++ _NOFOLLOW_DIRECTORY_FLAGS, ++ dir_fd=current_fd, ++ ) ++ except Exception: ++ try: ++ os.rmdir(component, dir_fd=current_fd) ++ os.fsync(current_fd) ++ except OSError: ++ pass ++ raise ++ except OSError as exc: ++ if exc.errno in (errno.ELOOP, errno.ENOTDIR): ++ raise ValueError( ++ f"output directory component is not a no-follow directory: {component}" ++ ) from exc ++ raise ++ if current_owned: ++ os.close(current_fd) ++ current_fd = next_fd ++ current_owned = True ++ if final: ++ created_root = created ++ except Exception: ++ if current_owned: ++ os.close(current_fd) ++ os.close(anchor_fd) ++ if root_parent_fd is not None: ++ os.close(root_parent_fd) ++ raise ++ os.close(anchor_fd) ++ root_parent_fd = cast(int, root_parent_fd) ++ root_name = cast(str, root_name) ++ ++ self._root_fd = current_fd ++ self._root_parent_fd = root_parent_fd ++ self._root_name = root_name ++ self._created_root = created_root ++ self._closed = False ++ self._known_directories: set[tuple[str, ...]] = {()} ++ self._created_directories: list[tuple[str, ...]] = [] ++ self._created_files: list[tuple[tuple[str, ...], str]] = [] ++ ++ def __enter__(self) -> Self: ++ """Return this open secure tree.""" ++ return self ++ ++ def __exit__( ++ self, ++ exc_type: type[BaseException] | None, ++ exc: BaseException | None, ++ traceback: TracebackType | None, ++ ) -> None: ++ """Clean owned partial output on failure and close all descriptors.""" ++ if exc_type is not None: ++ self.cleanup() ++ self.close() ++ ++ def _ensure_open(self) -> None: ++ """Fail when an operation is attempted after close.""" ++ if self._closed: ++ raise RuntimeError("secure output tree is closed") ++ ++ def _open_directory(self, parts: Iterable[str], *, create: bool) -> int: ++ """Open one descendant directory by walking held descriptors only.""" ++ self._ensure_open() ++ current_fd = os.dup(self._root_fd) ++ prefix: tuple[str, ...] = () ++ try: ++ for component in parts: ++ prefix = (*prefix, component) ++ if prefix not in self._known_directories: ++ if not create: ++ raise FileNotFoundError("secure output directory is unknown") ++ try: ++ os.mkdir(component, mode=0o700, dir_fd=current_fd) ++ except FileExistsError as exc: ++ raise FileExistsError( ++ f"secure output entry already exists: {'/'.join(prefix)}" ++ ) from exc ++ os.fsync(current_fd) ++ self._known_directories.add(prefix) ++ self._created_directories.append(prefix) ++ try: ++ next_fd = os.open( ++ component, ++ _NOFOLLOW_DIRECTORY_FLAGS, ++ dir_fd=current_fd, ++ ) ++ except OSError as exc: ++ if exc.errno in (errno.ELOOP, errno.ENOTDIR): ++ raise ValueError( ++ f"secure output directory was replaced or is not a directory: {'/'.join(prefix)}" ++ ) from exc ++ \ No newline at end of file diff --git a/docs/superpowers/patches/2026-08-07-npm-descriptor-safe-output.part02 b/docs/superpowers/patches/2026-08-07-npm-descriptor-safe-output.part02 new file mode 100644 index 000000000..5cc912478 --- /dev/null +++ b/docs/superpowers/patches/2026-08-07-npm-descriptor-safe-output.part02 @@ -0,0 +1,205 @@ + raise ++ os.close(current_fd) ++ current_fd = next_fd ++ return current_fd ++ except Exception: ++ os.close(current_fd) ++ raise ++ ++ def mkdir(self, relative_dir: str | pathlib.PurePosixPath) -> None: ++ """Create one descendant directory tree exclusively and durably.""" ++ parts = _relative_parts(relative_dir) ++ descriptor = self._open_directory(parts, create=True) ++ try: ++ os.fsync(descriptor) ++ finally: ++ os.close(descriptor) ++ ++ def write_bytes( ++ self, ++ relative_path: str | pathlib.PurePosixPath, ++ content: bytes, ++ ) -> None: ++ """Create one file exclusively, write all bytes, and fsync it and its parent.""" ++ if not isinstance(content, bytes): ++ raise TypeError("secure output content must be bytes") ++ parts = _relative_parts(relative_path) ++ parent_parts, file_name = parts[:-1], parts[-1] ++ parent_fd = self._open_directory(parent_parts, create=True) ++ file_fd: int | None = None ++ created = False ++ try: ++ try: ++ file_fd = os.open(file_name, _FILE_FLAGS, 0o600, dir_fd=parent_fd) ++ except FileExistsError as exc: ++ raise FileExistsError( ++ f"secure output file already exists: {'/'.join(parts)}" ++ ) from exc ++ created = True ++ _write_all(file_fd, content) ++ os.fsync(file_fd) ++ os.close(file_fd) ++ file_fd = None ++ os.fsync(parent_fd) ++ self._created_files.append((parent_parts, file_name)) ++ except Exception: ++ if file_fd is not None: ++ os.close(file_fd) ++ if created: ++ try: ++ os.unlink(file_name, dir_fd=parent_fd) ++ os.fsync(parent_fd) ++ except OSError: ++ pass ++ raise ++ finally: ++ os.close(parent_fd) ++ ++ def write_text( ++ self, ++ relative_path: str | pathlib.PurePosixPath, ++ content: str, ++ ) -> None: ++ """Encode UTF-8 text and create it through :meth:`write_bytes`.""" ++ if not isinstance(content, str): ++ raise TypeError("secure output text must be a string") ++ self.write_bytes(relative_path, content.encode("utf-8")) ++ ++ def _open_known_parent(self, parts: tuple[str, ...]) -> int | None: ++ """Reopen a known directory without following replacements for cleanup.""" ++ try: ++ return self._open_directory(parts, create=False) ++ except (OSError, RuntimeError, ValueError): ++ return None ++ ++ def cleanup(self) -> None: ++ """Remove only files and directories created by this instance.""" ++ if self._closed: ++ return ++ for parent_parts, file_name in reversed(self._created_files): ++ parent_fd = self._open_known_parent(parent_parts) ++ if parent_fd is None: ++ continue ++ try: ++ try: ++ os.unlink(file_name, dir_fd=parent_fd) ++ os.fsync(parent_fd) ++ except OSError: ++ pass ++ finally: ++ os.close(parent_fd) ++ self._created_files.clear() ++ ++ for parts in sorted(self._created_directories, key=len, reverse=True): ++ parent_fd = self._open_known_parent(parts[:-1]) ++ if parent_fd is None: ++ continue ++ try: ++ try: ++ os.rmdir(parts[-1], dir_fd=parent_fd) ++ os.fsync(parent_fd) ++ except OSError: ++ pass ++ finally: ++ os.close(parent_fd) ++ self._known_directories.discard(parts) ++ self._created_directories.clear() ++ ++ def close(self) -> None: ++ """Close descriptors and remove an empty root owned by this instance.""" ++ if self._closed: ++ return ++ self._closed = True ++ try: ++ os.fsync(self._root_fd) ++ finally: ++ os.close(self._root_fd) ++ if self._created_root: ++ try: ++ os.rmdir(self._root_name, dir_fd=self._root_parent_fd) ++ os.fsync(self._root_parent_fd) ++ except OSError: ++ pass ++ os.close(self._root_parent_fd) +diff --git a/tests/test_secure_output_tree.py b/tests/test_secure_output_tree.py +new file mode 100644 +index 0000000..3c4a8e5 +--- /dev/null ++++ b/tests/test_secure_output_tree.py +@@ -0,0 +1,363 @@ ++"""Security and failure-contract tests for descriptor-relative output writes.""" ++ ++from __future__ import annotations ++ ++import importlib.util ++import os ++from pathlib import Path ++ ++import pytest ++ ++ ++MODULE_PATH = Path(__file__).parents[1] / "scripts" / "ci" / "secure_output_tree.py" ++SPEC = importlib.util.spec_from_file_location("secure_output_tree", MODULE_PATH) ++assert SPEC is not None and SPEC.loader is not None ++module = importlib.util.module_from_spec(SPEC) ++SPEC.loader.exec_module(module) ++SecureOutputTree = module.SecureOutputTree ++ ++ ++@pytest.mark.skipif(os.name != "posix", reason="secure writer is intentionally POSIX-only") ++def test_secure_tree_writes_nested_bytes_and_text(tmp_path: Path) -> None: ++ """Owned files are written beneath a descriptor-anchored tree.""" ++ output = tmp_path / "output" ++ with SecureOutputTree(output) as tree: ++ tree.mkdir("project-000") ++ tree.write_bytes("project-000/package.json", b"{}\n") ++ tree.write_text("manifest.json", "[]\n") ++ ++ assert (output / "project-000" / "package.json").read_bytes() == b"{}\n" ++ assert (output / "manifest.json").read_text(encoding="utf-8") == "[]\n" ++ ++ ++@pytest.mark.skipif(os.name != "posix", reason="secure writer is intentionally POSIX-only") ++@pytest.mark.parametrize("path", ["../escape", "safe/../escape", "/absolute", "bad\\path", "a//b", "./x"]) ++def test_secure_tree_rejects_nonlexical_relative_paths(tmp_path: Path, path: str) -> None: ++ """No normalization may erase an absolute, parent, empty, or dot component.""" ++ with SecureOutputTree(tmp_path / "output") as tree: ++ with pytest.raises(ValueError): ++ tree.write_bytes(path, b"x") ++ ++ ++@pytest.mark.skipif(os.name != "posix", reason="secure writer is intentionally POSIX-only") ++def test_root_rename_and_symlink_swap_cannot_redirect_write(tmp_path: Path) -> None: ++ """A swap after root open writes to the held directory, never the attacker path.""" ++ output = tmp_path / "output" ++ moved = tmp_path / "moved" ++ attacker = tmp_path / "attacker" ++ attacker.mkdir() ++ tree = SecureOutputTree(output) ++ tree.mkdir("project-000") ++ output.rename(moved) ++ output.symlink_to(attacker, target_is_directory=True) ++ try: ++ tree.write_bytes("project-000/package.json", b"trusted") ++ finally: ++ tree.close() ++ assert (moved / "project-000" / "package.json").read_bytes() == b"trusted" ++ assert not (attacker / "project-000" / "package.json").exists() ++ ++ ++@pytest.mark.skipif(os.name != "posix", reason="secure writer is intentionally POSIX-only") ++def test_nested_directory_swap_fails_closed(tmp_path: Path) -> None: ++ """A nested directory replaced by a symlink is rejected before file creation.""" ++ output = tmp_path / "output" ++ attacker = tmp_path / "attacker" ++ attacker.mkdir() ++ with SecureOutputTree(output) as tree: ++ tree.mkdir("project-000") ++ (output / "project-000").rename(output / "project-moved") ++ (output / "project-000").symlink_to(attacker, target_is_directory=True) ++ with pytest.raises((OSError, ValueError)): ++ tree.write_bytes("project-000/package.json", b"trusted") ++ assert not (attacker / "package.json").exists() ++ ++ ++@pytest.mark.ski \ No newline at end of file diff --git a/docs/superpowers/patches/2026-08-07-npm-descriptor-safe-output.part03 b/docs/superpowers/patches/2026-08-07-npm-descriptor-safe-output.part03 new file mode 100644 index 000000000..9987435f2 --- /dev/null +++ b/docs/superpowers/patches/2026-08-07-npm-descriptor-safe-output.part03 @@ -0,0 +1,166 @@ +pif(os.name != "posix", reason="secure writer is intentionally POSIX-only") ++def test_preexisting_output_file_is_never_overwritten(tmp_path: Path) -> None: ++ """Exclusive creation preserves every pre-existing output entry.""" ++ output = tmp_path / "output" ++ output.mkdir() ++ existing = output / "manifest.json" ++ existing.write_text("attacker\n", encoding="utf-8") ++ with SecureOutputTree(output) as tree: ++ with pytest.raises(FileExistsError): ++ tree.write_text("manifest.json", "trusted\n") ++ assert existing.read_text(encoding="utf-8") == "attacker\n" ++ ++ ++@pytest.mark.skipif(os.name != "posix", reason="secure writer is intentionally POSIX-only") ++def test_preexisting_nested_directory_is_not_adopted(tmp_path: Path) -> None: ++ """A pre-existing descendant cannot be silently adopted into the owned tree.""" ++ output = tmp_path / "output" ++ output.mkdir() ++ (output / "project-000").mkdir() ++ with SecureOutputTree(output) as tree: ++ with pytest.raises(FileExistsError): ++ tree.mkdir("project-000") ++ ++ ++@pytest.mark.skipif(os.name != "posix", reason="secure writer is intentionally POSIX-only") ++def test_stalled_write_fails_and_removes_partial_file(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: ++ """A zero-progress write is an error and leaves no partial owned file.""" ++ output = tmp_path / "output" ++ real_write = module.os.write ++ ++ def stalled_write(file_descriptor: int, content: bytes) -> int: ++ del file_descriptor, content ++ return 0 ++ ++ monkeypatch.setattr(module.os, "write", stalled_write) ++ with SecureOutputTree(output) as tree: ++ with pytest.raises(OSError, match="no forward progress"): ++ tree.write_bytes("project-000/package.json", b"trusted") ++ monkeypatch.setattr(module.os, "write", real_write) ++ assert not (output / "project-000" / "package.json").exists() ++ ++ ++@pytest.mark.skipif(os.name != "posix", reason="secure writer is intentionally POSIX-only") ++def test_failure_cleans_only_owned_entries(tmp_path: Path) -> None: ++ """An existing empty root survives while created descendants are removed.""" ++ output = tmp_path / "output" ++ output.mkdir() ++ with pytest.raises(RuntimeError, match="boom"): ++ with SecureOutputTree(output) as tree: ++ tree.write_bytes("project-000/package.json", b"trusted") ++ raise RuntimeError("boom") ++ assert output.is_dir() ++ assert list(output.iterdir()) == [] ++ ++ ++@pytest.mark.skipif(os.name != "posix", reason="secure writer is intentionally POSIX-only") ++def test_failure_removes_owned_root_when_empty(tmp_path: Path) -> None: ++ """A root created by the helper is removed after owned partial cleanup.""" ++ output = tmp_path / "output" ++ with pytest.raises(RuntimeError, match="boom"): ++ with SecureOutputTree(output) as tree: ++ tree.write_bytes("project-000/package.json", b"trusted") ++ raise RuntimeError("boom") ++ assert not output.exists() ++ ++ ++def test_missing_secure_capabilities_fail_closed(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: ++ """Unsupported runtimes never fall back to mutable pathname writes.""" ++ monkeypatch.setattr(module, "_secure_primitives_available", lambda: False) ++ with pytest.raises(RuntimeError, match="requires POSIX"): ++ SecureOutputTree(tmp_path / "output") ++ ++ ++@pytest.mark.skipif(os.name != "posix", reason="secure writer is intentionally POSIX-only") ++def test_missing_parent_component_fails_without_creating_ancestors(tmp_path: Path) -> None: ++ """Only the final output root may be created; missing ancestors fail closed.""" ++ output = tmp_path / "missing" / "output" ++ with pytest.raises(ValueError, match="parent components"): ++ SecureOutputTree(output) ++ assert not (tmp_path / "missing").exists() ++ ++ ++@pytest.mark.skipif(os.name != "posix", reason="secure writer is intentionally POSIX-only") ++def test_closed_tree_rejects_operations_and_close_is_idempotent(tmp_path: Path) -> None: ++ """Closed descriptors cannot be reused and repeated cleanup is harmless.""" ++ tree = SecureOutputTree(tmp_path / "output") ++ tree.close() ++ tree.close() ++ tree.cleanup() ++ with pytest.raises(RuntimeError, match="closed"): ++ tree.mkdir("project-000") ++ ++ ++@pytest.mark.skipif(os.name != "posix", reason="secure writer is intentionally POSIX-only") ++def test_output_root_and_parent_components_are_validated(tmp_path: Path) -> None: ++ """Parent traversal, root output, symbolic links, and regular files fail closed.""" ++ with pytest.raises(ValueError, match="parent components"): ++ SecureOutputTree(Path("safe/../output")) ++ with pytest.raises(ValueError, match="filesystem root"): ++ SecureOutputTree(Path("/")) ++ ++ attacker = tmp_path / "attacker" ++ attacker.mkdir() ++ symlink_parent = tmp_path / "link" ++ symlink_parent.symlink_to(attacker, target_is_directory=True) ++ with pytest.raises(ValueError, match="no-follow directory"): ++ SecureOutputTree(symlink_parent / "output") ++ ++ regular_parent = tmp_path / "regular" ++ regular_parent.write_text("not a directory", encoding="utf-8") ++ with pytest.raises(ValueError, match="no-follow directory"): ++ SecureOutputTree(regular_parent / "output") ++ ++ ++@pytest.mark.skipif(os.name != "posix", reason="secure writer is intentionally POSIX-only") ++def test_secure_tree_rejects_wrong_content_types_and_empty_paths(tmp_path: Path) -> None: ++ """Public write methods accept only declared types and nonempty paths.""" ++ with SecureOutputTree(tmp_path / "output") as tree: ++ with pytest.raises(ValueError, match="must not be empty"): ++ tree.write_bytes("", b"x") ++ with pytest.raises(TypeError, match="must be bytes"): ++ tree.write_bytes("value.bin", "not-bytes") # type: ignore[arg-type] ++ with pytest.raises(TypeError, match="must be a string"): ++ tree.write_text("value.txt", b"not-text") # type: ignore[arg-type] ++ with pytest.raises(FileNotFoundError, match="unknown"): ++ descriptor = tree._open_directory(("unknown",), create=False) ++ os.close(descriptor) ++ ++ ++@pytest.mark.skipif(os.name != "posix", reason="secure writer is intentionally POSIX-only") ++def test_constructor_reopen_race_cleans_created_root(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: ++ """Failure reopening a newly created root removes the directory and closes safely.""" ++ output = tmp_path / "output" ++ real_open = module.os.open ++ ++ def failing_reopen(path, flags, mode=0o777, *, dir_fd=None): ++ if path == "output" and output.exists(): ++ raise OSError(5, "injected reopen failure") ++ return real_open(path, flags, mode, dir_fd=dir_fd) ++ ++ monkeypatch.setattr(module, "_secure_primitives_available", lambda: True) ++ monkeypatch.setattr(module.os, "open", failing_reopen) ++ with pytest.raises(OSError, match="injected reopen"): ++ SecureOutputTree(output) ++ assert not output.exists() ++ ++ ++@pytest.mark.skipif(os.name != "posix", reason="secure writer is intentionally POSIX-only") ++def test_constructor_propagates_non_path_os_errors(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: ++ """Unexpected descriptor errors are propagated rather than recast as path validation.""" ++ output = tmp_path / "output" ++ output.mkdir() ++ real_open = module.os.open ++ ++ def denied_open(path, flags, mode=0o777, *, dir_fd=None): ++ if path == "output" and dir_fd is not None: ++ raise PermissionError(13, "injected denial") ++ return real_open(path, flags, mode, dir_fd=dir_fd) ++ ++ monkeypatch.setattr(module, "_secure_primitives_available", lambda: True) ++ monkeypatch.setattr(module.os, "open", denied_open) ++ with pytest.raises(PermissionError, match="injected denial"): ++ SecureOutputTree(output) ++ ++ ++@pytest.mark.skipif( \ No newline at end of file diff --git a/docs/superpowers/patches/2026-08-07-npm-descriptor-safe-output.part04 b/docs/superpowers/patches/2026-08-07-npm-descriptor-safe-output.part04 new file mode 100644 index 000000000..d3e199e9e --- /dev/null +++ b/docs/superpowers/patches/2026-08-07-npm-descriptor-safe-output.part04 @@ -0,0 +1,123 @@ +os.name != "posix", reason="secure writer is intentionally POSIX-only") ++def test_descendant_open_propagates_unexpected_os_error(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: ++ """An unexpected descendant open error closes its current descriptor and propagates.""" ++ output = tmp_path / "output" ++ tree = SecureOutputTree(output) ++ tree.mkdir("project-000") ++ real_open = module.os.open ++ ++ def denied_open(path, flags, mode=0o777, *, dir_fd=None): ++ if path == "project-000" and dir_fd is not None: ++ raise PermissionError(13, "injected descendant denial") ++ return real_open(path, flags, mode, dir_fd=dir_fd) ++ ++ monkeypatch.setattr(module, "_secure_primitives_available", lambda: True) ++ monkeypatch.setattr(module.os, "open", denied_open) ++ try: ++ with pytest.raises(PermissionError, match="injected descendant denial"): ++ tree.write_bytes("project-000/package.json", b"trusted") ++ finally: ++ tree.close() ++ ++ ++@pytest.mark.skipif(os.name != "posix", reason="secure writer is intentionally POSIX-only") ++def test_failed_write_tolerates_cleanup_unlink_failure(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: ++ """The original write error remains authoritative when best-effort unlink also fails.""" ++ output = tmp_path / "output" ++ monkeypatch.setattr(module, "_secure_primitives_available", lambda: True) ++ monkeypatch.setattr(module.os, "write", lambda _fd, _content: 0) ++ monkeypatch.setattr( ++ module.os, ++ "unlink", ++ lambda *_args, **_kwargs: (_ for _ in ()).throw(OSError("unlink denied")), ++ ) ++ with SecureOutputTree(output) as tree: ++ with pytest.raises(OSError, match="no forward progress"): ++ tree.write_bytes("project-000/package.json", b"trusted") ++ ++ ++@pytest.mark.skipif(os.name != "posix", reason="secure writer is intentionally POSIX-only") ++def test_cleanup_skips_replaced_parents_without_following_them(tmp_path: Path) -> None: ++ """Cleanup skips files and nested directories whose owned parent was replaced.""" ++ output = tmp_path / "output" ++ attacker = tmp_path / "attacker" ++ attacker.mkdir() ++ tree = SecureOutputTree(output) ++ tree.write_bytes("project-000/sub/package.json", b"trusted") ++ (output / "project-000").rename(output / "project-moved") ++ (output / "project-000").symlink_to(attacker, target_is_directory=True) ++ try: ++ tree.cleanup() ++ finally: ++ tree.close() ++ assert not (attacker / "sub" / "package.json").exists() ++ assert (output / "project-moved" / "sub" / "package.json").read_bytes() == b"trusted" ++ ++ ++@pytest.mark.skipif(os.name != "posix", reason="secure writer is intentionally POSIX-only") ++def test_cleanup_tolerates_unlink_and_rmdir_failures(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: ++ """Cleanup never deletes unowned data and tolerates operating-system cleanup refusal.""" ++ output = tmp_path / "output" ++ tree = SecureOutputTree(output) ++ tree.write_bytes("project-000/package.json", b"trusted") ++ monkeypatch.setattr( ++ module.os, ++ "unlink", ++ lambda *_args, **_kwargs: (_ for _ in ()).throw(OSError("unlink denied")), ++ ) ++ monkeypatch.setattr( ++ module.os, ++ "rmdir", ++ lambda *_args, **_kwargs: (_ for _ in ()).throw(OSError("rmdir denied")), ++ ) ++ try: ++ tree.cleanup() ++ finally: ++ tree.close() ++ ++ ++def test_open_known_parent_returns_none_after_close(tmp_path: Path) -> None: ++ """Cleanup lookup treats a closed descriptor as unavailable rather than reopening paths.""" ++ if os.name != "posix": ++ pytest.skip("secure writer is intentionally POSIX-only") ++ tree = SecureOutputTree(tmp_path / "output") ++ tree.close() ++ assert tree._open_known_parent(()) is None ++ ++ ++@pytest.mark.skipif(os.name != "posix", reason="secure writer is intentionally POSIX-only") ++def test_constructor_reopen_race_tolerates_root_cleanup_failure(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: ++ """The original reopen error survives a best-effort newly-created-root cleanup failure.""" ++ output = tmp_path / "output" ++ real_open = module.os.open ++ ++ def failing_reopen(path, flags, mode=0o777, *, dir_fd=None): ++ if path == "output" and output.exists(): ++ raise OSError(5, "injected reopen failure") ++ return real_open(path, flags, mode, dir_fd=dir_fd) ++ ++ monkeypatch.setattr(module, "_secure_primitives_available", lambda: True) ++ monkeypatch.setattr(module.os, "open", failing_reopen) ++ monkeypatch.setattr( ++ module.os, ++ "rmdir", ++ lambda *_args, **_kwargs: (_ for _ in ()).throw(OSError("cleanup denied")), ++ ) ++ with pytest.raises(OSError, match="injected reopen"): ++ SecureOutputTree(output) ++ ++ ++@pytest.mark.skipif(os.name != "posix", reason="secure writer is intentionally POSIX-only") ++def test_constructor_failure_before_first_descendant_open_closes_anchor(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: ++ """A first-component failure exercises the unowned-current-descriptor cleanup path.""" ++ real_open = module.os.open ++ ++ def denied_first_component(path, flags, mode=0o777, *, dir_fd=None): ++ if path == "tmp" and dir_fd is not None: ++ raise PermissionError(13, "injected first-component denial") ++ return real_open(path, flags, mode, dir_fd=dir_fd) ++ ++ monkeypatch.setattr(module, "_secure_primitives_available", lambda: True) ++ monkeypatch.setattr(module.os, "open", denied_first_component) ++ with pytest.raises(PermissionError, match="first-component denial"): ++ SecureOutputTree(tmp_path / "output") diff --git a/scripts/ci/materialize_base_javascript_packages.py b/scripts/ci/materialize_base_javascript_packages.py index 407c17aa1..14b4775cf 100644 --- a/scripts/ci/materialize_base_javascript_packages.py +++ b/scripts/ci/materialize_base_javascript_packages.py @@ -12,8 +12,10 @@ import argparse import json +import os import pathlib import re +import stat import subprocess import sys import urllib.parse @@ -26,6 +28,11 @@ NPM_LOCK_NAMES = ("npm-shrinkwrap.json", "package-lock.json") NPM_REGISTRY_HOST = "registry.npmjs.org" SHA512_SRI_RE = re.compile(r"^sha512-[A-Za-z0-9+/]{86}==$") +_DIRECTORY_OPEN_FLAGS = os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW +_NEW_FILE_FLAGS = os.O_WRONLY | os.O_CREAT | os.O_EXCL | os.O_NOFOLLOW +_REQUIRED_DIR_FD_FUNCTIONS = (os.open, os.mkdir, os.stat, os.unlink, os.rmdir) +_REQUIRED_FD_FUNCTIONS = (os.listdir,) +_REQUIRED_FOLLOW_SYMLINK_FUNCTIONS = (os.stat,) def _git(repo_root: pathlib.Path, *args: str) -> bytes: @@ -112,12 +119,6 @@ def base_pnpm_projects( str(project_root / lock_name) in regular_paths for lock_name in NPM_LOCK_NAMES ): - # A sibling npm lock means npm owns this project and the - # pnpm-lock.yaml is a vestigial second lockfile. Skip pnpm - # materialization so the downstream npm install path handles - # it, instead of failing the whole coverage-evidence job. A - # genuine pnpm-only project (no sibling npm lock) still must - # pin an exact pnpm packageManager. continue raise ValueError( f"trusted base package manifest {package_path} must declare an exact pnpm packageManager version" @@ -194,8 +195,6 @@ def base_npm_projects( ) package_manager = package_data.get("packageManager") if isinstance(package_manager, str) and PNPM_SPEC_RE.fullmatch(package_manager): - # An exact pnpm declaration owns this project. A sibling npm lock - # is vestigial and must not create a second dependency cache. continue lock_content = _git(repo_root, "show", f"{base_sha}:{lock_path}") @@ -249,6 +248,73 @@ def _lock_blob_sha(repo_root: pathlib.Path, revision_sha: str, lock_path: str) - return blob_sha.lower() +def _npm_package_identity( + lock_path: str, + package_path: str, + candidate: pathlib.PurePosixPath, +) -> str: + """Return the exact npm identity after the final ``node_modules`` segment.""" + final_node_modules = max( + index for index, part in enumerate(candidate.parts) if part == "node_modules" + ) + identity_parts = candidate.parts[final_node_modules + 1 :] + if ( + len(identity_parts) == 1 + and identity_parts[0] + and not identity_parts[0].startswith("@") + ): + return identity_parts[0] + if ( + len(identity_parts) == 2 + and identity_parts[0].startswith("@") + and len(identity_parts[0]) > 1 + and identity_parts[1] + and not identity_parts[1].startswith("@") + ): + return "/".join(identity_parts) + raise ValueError( + f"current-head npm lock {lock_path} package {package_path} has a malformed npm package identity" + ) + + +def _validate_npm_registry_pin( + lock_path: str, + package_path: str, + resolved: Any, + integrity: Any, +) -> None: + """Validate one exact public-registry tarball and SHA-512 integrity pair.""" + if not isinstance(resolved, str) or not isinstance(integrity, str): + raise ValueError( + f"current-head npm lock {lock_path} package {package_path} must pin a registry tarball and SHA-512 integrity" + ) + parsed = urllib.parse.urlsplit(resolved) + try: + parsed_port = parsed.port + except ValueError as exc: + raise ValueError( + f"current-head npm lock {lock_path} package {package_path} has an invalid registry URL" + ) from exc + if ( + parsed.scheme != "https" + or parsed.hostname != NPM_REGISTRY_HOST + or parsed.username is not None + or parsed.password is not None + or parsed_port is not None + or parsed.query + or parsed.fragment + or not parsed.path.startswith("/") + or not parsed.path.endswith(".tgz") + ): + raise ValueError( + f"current-head npm lock {lock_path} package {package_path} must resolve from https://{NPM_REGISTRY_HOST}/" + ) + if not SHA512_SRI_RE.fullmatch(integrity): + raise ValueError( + f"current-head npm lock {lock_path} package {package_path} must use one SHA-512 integrity value" + ) + + def validate_head_npm_lock(lock_path: str, lock_content: bytes) -> None: """Fail closed unless a changed HEAD npm lock is registry- and hash-bounded.""" try: @@ -274,6 +340,8 @@ def validate_head_npm_lock(lock_path: str, lock_content: bytes) -> None: f"current-head npm lock {lock_path} must contain an object-valued packages map" ) + canonical_versions: dict[str, str] = {} + metadata_only_locations: list[tuple[str, str, str]] = [] for package_path, metadata in sorted(packages.items()): if not isinstance(package_path, str) or not isinstance(metadata, dict): raise ValueError( @@ -291,6 +359,7 @@ def validate_head_npm_lock(lock_path: str, lock_content: bytes) -> None: if not package_path or "node_modules" not in candidate.parts: continue + identity = _npm_package_identity(lock_path, package_path, candidate) resolved = metadata.get("resolved") if metadata.get("link") is True: if not isinstance(resolved, str) or not resolved or "\\" in resolved: @@ -308,36 +377,343 @@ def validate_head_npm_lock(lock_path: str, lock_content: bytes) -> None: ) continue - integrity = metadata.get("integrity") - if not isinstance(resolved, str) or not isinstance(integrity, str): + has_resolved = "resolved" in metadata + has_integrity = "integrity" in metadata + if has_resolved != has_integrity: raise ValueError( - f"current-head npm lock {lock_path} package {package_path} must pin a registry tarball and SHA-512 integrity" + f"current-head npm lock {lock_path} package {package_path} must not partially declare a registry tarball and SHA-512 integrity" ) - parsed = urllib.parse.urlsplit(resolved) + if has_resolved: + _validate_npm_registry_pin( + lock_path, + package_path, + metadata.get("resolved"), + metadata.get("integrity"), + ) + + version = metadata.get("version") + canonical_path = f"node_modules/{identity}" + is_canonical_root = package_path == canonical_path + if not isinstance(version, str) or not version: + if is_canonical_root and not has_resolved: + raise ValueError( + f"current-head npm lock {lock_path} package {package_path} must pin a registry tarball and SHA-512 integrity" + ) + raise ValueError( + f"current-head npm lock {lock_path} package {package_path} must declare a nonempty exact version" + ) + + if has_resolved: + if is_canonical_root: + canonical_versions[identity] = version + continue + + if is_canonical_root: + raise ValueError( + f"current-head npm lock {lock_path} package {package_path} must be a complete canonical root pin" + ) + metadata_only_locations.append((package_path, identity, version)) + + for package_path, identity, version in metadata_only_locations: + canonical_version = canonical_versions.get(identity) + if canonical_version is None: + raise ValueError( + f"current-head npm lock {lock_path} package {package_path} has no complete canonical root pin" + ) + if canonical_version != version: + raise ValueError( + f"current-head npm lock {lock_path} package {package_path} must match the exact canonical version {canonical_version}" + ) + + +def _require_descriptor_relative_capabilities() -> None: + """Fail before mutation when required descriptor-relative filesystem APIs are absent.""" + dir_fd_supported = getattr(os, "supports_dir_fd", set()) + fd_supported = getattr(os, "supports_fd", set()) + follow_symlinks_supported = getattr(os, "supports_follow_symlinks", set()) + if ( + any( + function not in dir_fd_supported + for function in _REQUIRED_DIR_FD_FUNCTIONS + ) + or any(function not in fd_supported for function in _REQUIRED_FD_FUNCTIONS) + or any( + function not in follow_symlinks_supported + for function in _REQUIRED_FOLLOW_SYMLINK_FUNCTIONS + ) + ): + raise ValueError("descriptor-relative output operations are unavailable") + if not all(hasattr(os, name) for name in ("O_DIRECTORY", "O_NOFOLLOW")): + raise ValueError("descriptor-relative output operations are unavailable") + + +def _reject_symlinked_output_components(output_dir: pathlib.Path) -> None: + """Reject existing symlink components before materialization writes begin.""" + candidate = output_dir.absolute() + if candidate == pathlib.Path(candidate.anchor): + raise ValueError("output directory must not be the filesystem root") + current = pathlib.Path(candidate.anchor) + for component in candidate.parts[1:]: + current /= component + if current.is_symlink(): + raise ValueError( + "output directory must not be a symlink; " + f"path must not contain symlinks: {current}" + ) + if not current.exists(): + break + if not current.is_dir(): + raise ValueError( + f"output directory path component must be a directory: {current}" + ) + + +def _directory_identity(metadata: os.stat_result) -> tuple[int, int]: + """Return one directory device/inode identity after validating its file type.""" + if not stat.S_ISDIR(metadata.st_mode): + raise ValueError("output directory binding changed during secure materialization") + return metadata.st_dev, metadata.st_ino + + +def _open_output_directory(output_dir: pathlib.Path) -> tuple[int, tuple[int, int]]: + """Open one no-follow output directory while detecting ancestor replacement races.""" + candidate = output_dir.absolute() + _reject_symlinked_output_components(candidate) + anchor = pathlib.Path(candidate.anchor) + anchor_fd = os.open(anchor, _DIRECTORY_OPEN_FLAGS) + try: + output_fd = _open_relative_directory(anchor_fd, tuple(candidate.parts[1:])) try: - parsed_port = parsed.port - except ValueError as exc: + os.fsync(output_fd) + metadata = os.fstat(output_fd) + identity = (metadata.st_dev, metadata.st_ino) + _verify_output_directory_binding(candidate, output_fd, identity) + return output_fd, identity + except BaseException: + os.close(output_fd) + raise + finally: + os.close(anchor_fd) + + +def _verify_output_directory_binding( + output_dir: pathlib.Path, + output_fd: int, + identity: tuple[int, int], +) -> None: + """Fail closed if the published output pathname no longer names the opened directory.""" + descriptor_metadata = os.fstat(output_fd) + try: + path_metadata = os.stat(output_dir.absolute(), follow_symlinks=False) + except OSError as exc: + raise ValueError("output directory changed during secure materialization") from exc + if ( + not stat.S_ISDIR(path_metadata.st_mode) + or (descriptor_metadata.st_dev, descriptor_metadata.st_ino) != identity + or (path_metadata.st_dev, path_metadata.st_ino) != identity + ): + raise ValueError("output directory changed during secure materialization") + + +def _safe_relative_parts(relative_path: str) -> tuple[str, ...]: + """Return one normalized relative POSIX output path or fail closed.""" + candidate = pathlib.PurePosixPath(relative_path) + if ( + not relative_path + or "\\" in relative_path + or candidate.is_absolute() + or ".." in candidate.parts + or candidate.as_posix() != relative_path + or not candidate.parts + ): + raise ValueError(f"unsafe relative output path: {relative_path!r}") + return candidate.parts + + +def _open_relative_directory(root_fd: int, parts: tuple[str, ...]) -> int: + """Open or create child directories and bind each name to its observed inode.""" + current_fd = os.dup(root_fd) + try: + for part in parts: + created = False + try: + os.mkdir(part, mode=0o700, dir_fd=current_fd) + created = True + except FileExistsError: + pass + expected_identity = _directory_identity( + os.stat(part, dir_fd=current_fd, follow_symlinks=False) + ) + if created: + os.fsync(current_fd) + next_fd = os.open(part, _DIRECTORY_OPEN_FLAGS, dir_fd=current_fd) + try: + if _directory_identity(os.fstat(next_fd)) != expected_identity: + raise ValueError( + "output directory binding changed during secure materialization" + ) + os.fsync(next_fd) + except BaseException: + os.close(next_fd) + raise + os.close(current_fd) + current_fd = next_fd + return current_fd + except BaseException: + os.close(current_fd) + raise + + +def _create_project_directory(output_fd: int, directory: str) -> int: + """Create and bind a fresh project directory beneath the pinned output descriptor.""" + try: + os.mkdir(directory, mode=0o700, dir_fd=output_fd) + except FileExistsError as exc: + raise ValueError( + f"generated output path must not pre-exist: {directory}" + ) from exc + expected_identity = _directory_identity( + os.stat(directory, dir_fd=output_fd, follow_symlinks=False) + ) + os.fsync(output_fd) + project_fd = os.open(directory, _DIRECTORY_OPEN_FLAGS, dir_fd=output_fd) + try: + if _directory_identity(os.fstat(project_fd)) != expected_identity: raise ValueError( - f"current-head npm lock {lock_path} package {package_path} has an invalid registry URL" - ) from exc - if ( - parsed.scheme != "https" - or parsed.hostname != NPM_REGISTRY_HOST - or parsed.username is not None - or parsed.password is not None - or parsed_port is not None - or parsed.query - or parsed.fragment - or not parsed.path.startswith("/") - or not parsed.path.endswith(".tgz") - ): + "output directory binding changed during secure materialization" + ) + os.fsync(project_fd) + return project_fd + except BaseException: + os.close(project_fd) + raise + + +def _unlink_owned_file( + parent_fd: int, + filename: str, + identity: tuple[int, int], +) -> None: + """Remove one failed file only when its published name still identifies our inode.""" + try: + path_metadata = os.stat(filename, dir_fd=parent_fd, follow_symlinks=False) + except OSError: + return + if (path_metadata.st_dev, path_metadata.st_ino) != identity: + return + try: + os.unlink(filename, dir_fd=parent_fd) + except OSError: + return + os.fsync(parent_fd) + + +def _remove_owned_empty_directory( + parent_fd: int, + directory: str, + identity: tuple[int, int], +) -> None: + """Remove one empty generated directory only while its original inode is published.""" + try: + path_metadata = os.stat(directory, dir_fd=parent_fd, follow_symlinks=False) + except OSError: + return + if ( + not stat.S_ISDIR(path_metadata.st_mode) + or (path_metadata.st_dev, path_metadata.st_ino) != identity + ): + return + try: + os.rmdir(directory, dir_fd=parent_fd) + except OSError: + return + os.fsync(parent_fd) + + +def _remove_owned_directory_contents(directory_fd: int) -> None: + """Remove regular files and directories owned by one fresh project attempt.""" + for entry in sorted(os.listdir(directory_fd), reverse=True): + metadata = os.stat(entry, dir_fd=directory_fd, follow_symlinks=False) + identity = (metadata.st_dev, metadata.st_ino) + if stat.S_ISREG(metadata.st_mode): + _unlink_owned_file(directory_fd, entry, identity) + continue + if not stat.S_ISDIR(metadata.st_mode): + raise ValueError( + "unexpected output entry during secure materialization cleanup" + ) + child_fd = os.open(entry, _DIRECTORY_OPEN_FLAGS, dir_fd=directory_fd) + try: + if _directory_identity(os.fstat(child_fd)) != identity: + raise ValueError( + "output directory binding changed during secure materialization" + ) + _remove_owned_directory_contents(child_fd) + finally: + os.close(child_fd) + _remove_owned_empty_directory(directory_fd, entry, identity) + + +def _write_new_file(parent_fd: int, filename: str, content: bytes) -> None: + """Create, synchronize, revalidate, and clean up one descriptor-pinned file.""" + try: + file_fd = os.open( + filename, + _NEW_FILE_FLAGS, + 0o600, + dir_fd=parent_fd, + ) + except FileExistsError as exc: + raise ValueError( + f"generated output file must not pre-exist: {filename}" + ) from exc + initial_metadata = os.fstat(file_fd) + identity = (initial_metadata.st_dev, initial_metadata.st_ino) + try: + if not stat.S_ISREG(initial_metadata.st_mode) or initial_metadata.st_nlink != 1: raise ValueError( - f"current-head npm lock {lock_path} package {package_path} must resolve from https://{NPM_REGISTRY_HOST}/" + "generated output files must be singly linked regular files" ) - if not SHA512_SRI_RE.fullmatch(integrity): + view = memoryview(content) + offset = 0 + while offset < len(view): + written = os.write(file_fd, view[offset:]) + if written <= 0: + raise OSError("output write made no progress") + offset += written + os.fsync(file_fd) + final_metadata = os.fstat(file_fd) + path_metadata = os.stat(filename, dir_fd=parent_fd, follow_symlinks=False) + if ( + not stat.S_ISREG(path_metadata.st_mode) + or (final_metadata.st_dev, final_metadata.st_ino) + != (path_metadata.st_dev, path_metadata.st_ino) + ): + raise ValueError("output file changed during secure materialization") + if final_metadata.st_nlink != 1 or path_metadata.st_nlink != 1: raise ValueError( - f"current-head npm lock {lock_path} package {package_path} must use one SHA-512 integrity value" + "generated output files must remain singly linked regular files" ) + os.fsync(parent_fd) + except BaseException: + _unlink_owned_file(parent_fd, filename, identity) + raise + finally: + os.close(file_fd) + + +def _write_relative_file( + project_fd: int, + relative_path: str, + content: bytes, +) -> None: + """Write one validated project-relative input through pinned directories.""" + parts = _safe_relative_parts(relative_path) + parent_fd = _open_relative_directory(project_fd, tuple(parts[:-1])) + try: + _write_new_file(parent_fd, parts[-1], content) + finally: + os.close(parent_fd) def materialize( @@ -347,81 +723,103 @@ def materialize( head_sha: str | None = None, ) -> list[dict[str, str]]: """Write trusted base and bounded HEAD inputs under Docker-context-safe paths.""" - if output_dir.exists() and output_dir.is_symlink(): - raise ValueError("output directory must not be a symlink") - output_dir.mkdir(parents=True, exist_ok=True) - - manifest: list[dict[str, str]] = [] - projects: list[tuple[str, str, dict[str, bytes], str, str]] = [] - base_npm = base_npm_projects(repo_root, base_sha) - base_npm_paths = {source_path for source_path, _manager, _inputs in base_npm} - base_npm_blobs: dict[str, str] = {} - for source_path, package_manager, base_inputs in ( - base_pnpm_projects(repo_root, base_sha) + base_npm - ): - lock_blob = _lock_blob_sha(repo_root, base_sha, source_path) - projects.append( - ( - source_path, - package_manager, - base_inputs, - base_sha.lower(), - lock_blob, - ) - ) - if source_path in base_npm_paths: - base_npm_blobs[source_path] = lock_blob - - if head_sha is not None: - if not SHA_RE.fullmatch(head_sha): - raise ValueError("head SHA must be exactly 40 hexadecimal characters") - for source_path, package_manager, head_inputs in base_npm_projects( - repo_root, head_sha + _require_descriptor_relative_capabilities() + output_fd, output_identity = _open_output_directory(output_dir) + try: + manifest: list[dict[str, str]] = [] + projects: list[tuple[str, str, dict[str, bytes], str, str]] = [] + base_npm = base_npm_projects(repo_root, base_sha) + base_npm_paths = {source_path for source_path, _manager, _inputs in base_npm} + base_npm_blobs: dict[str, str] = {} + for source_path, package_manager, base_inputs in ( + base_pnpm_projects(repo_root, base_sha) + base_npm ): - head_blob = _lock_blob_sha(repo_root, head_sha, source_path) - if base_npm_blobs.get(source_path) == head_blob: - continue - lock_name = pathlib.PurePosixPath(source_path).name - validate_head_npm_lock(source_path, head_inputs[lock_name]) + lock_blob = _lock_blob_sha(repo_root, base_sha, source_path) projects.append( ( source_path, package_manager, - head_inputs, - head_sha.lower(), - head_blob, + base_inputs, + base_sha.lower(), + lock_blob, ) ) + if source_path in base_npm_paths: + base_npm_blobs[source_path] = lock_blob + + if head_sha is not None: + if not SHA_RE.fullmatch(head_sha): + raise ValueError("head SHA must be exactly 40 hexadecimal characters") + for source_path, package_manager, head_inputs in base_npm_projects( + repo_root, head_sha + ): + head_blob = _lock_blob_sha(repo_root, head_sha, source_path) + if base_npm_blobs.get(source_path) == head_blob: + continue + lock_name = pathlib.PurePosixPath(source_path).name + validate_head_npm_lock(source_path, head_inputs[lock_name]) + projects.append( + ( + source_path, + package_manager, + head_inputs, + head_sha.lower(), + head_blob, + ) + ) - for index, ( - source_path, - package_manager, - base_inputs, - revision_sha, - lock_blob, - ) in enumerate(sorted(projects, key=lambda project: (project[0], project[3]))): - directory = f"project-{index:03d}" - project_dir = output_dir / directory - project_dir.mkdir() - for relative_path, content in sorted(base_inputs.items()): - destination = project_dir / relative_path - destination.parent.mkdir(parents=True, exist_ok=True) - destination.write_bytes(content) - manifest.append( - { - "directory": directory, - "lock_blob": lock_blob, - "package_manager": package_manager, - "revision_sha": revision_sha, - "source": source_path, - } - ) + for index, ( + source_path, + package_manager, + base_inputs, + revision_sha, + lock_blob, + ) in enumerate(sorted(projects, key=lambda project: (project[0], project[3]))): + directory = f"project-{index:03d}" + project_fd = _create_project_directory(output_fd, directory) + project_metadata = os.fstat(project_fd) + project_identity = (project_metadata.st_dev, project_metadata.st_ino) + project_failed = False + try: + for relative_path, content in sorted(base_inputs.items()): + _write_relative_file(project_fd, relative_path, content) + os.fsync(project_fd) + except BaseException: + project_failed = True + try: + _remove_owned_directory_contents(project_fd) + except (OSError, ValueError): + # Preserve the first fail-closed boundary and leave any + # unowned or raced entry available for forensic inspection. + pass + raise + finally: + os.close(project_fd) + if project_failed: + _remove_owned_empty_directory( + output_fd, + directory, + project_identity, + ) + manifest.append( + { + "directory": directory, + "lock_blob": lock_blob, + "package_manager": package_manager, + "revision_sha": revision_sha, + "source": source_path, + } + ) - (output_dir / "manifest.json").write_text( - json.dumps(manifest, indent=2, sort_keys=True) + "\n", - encoding="utf-8", - ) - return manifest + manifest_content = ( + json.dumps(manifest, indent=2, sort_keys=True) + "\n" + ).encode("utf-8") + _write_new_file(output_fd, "manifest.json", manifest_content) + os.fsync(output_fd) + _verify_output_directory_binding(output_dir, output_fd, output_identity) + return manifest + finally: + os.close(output_fd) def main(argv: list[str] | None = None) -> int: diff --git a/scripts/ci/materialize_base_python_requirements.py b/scripts/ci/materialize_base_python_requirements.py index b16d4c745..c6cae9dee 100755 --- a/scripts/ci/materialize_base_python_requirements.py +++ b/scripts/ci/materialize_base_python_requirements.py @@ -15,6 +15,7 @@ import platform import re import shutil +import stat import subprocess import sys import tarfile @@ -63,6 +64,11 @@ TRUSTED_UV_ORIGIN_ERROR = ( "trusted uv archive redirected outside the fixed GitHub release HTTPS origin" ) +_DIRECTORY_OPEN_FLAGS = os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW +_NEW_FILE_FLAGS = os.O_WRONLY | os.O_CREAT | os.O_EXCL | os.O_NOFOLLOW +_REQUIRED_DIR_FD_FUNCTIONS = (os.open, os.mkdir, os.stat, os.unlink) +_REQUIRED_FD_FUNCTIONS = (os.listdir,) +_REQUIRED_FOLLOW_SYMLINK_FUNCTIONS = (os.stat,) def _https_default_port(parsed: urllib.parse.ParseResult) -> bool: @@ -258,8 +264,6 @@ def _is_flat_materializable_lock(content: bytes) -> bool: return bool(requirement_lines) and all( _is_fully_hash_pinned_requirement(line) for line in requirement_lines ) - - def _is_fully_hash_pinned_requirement(line: str) -> bool: """Return whether one uv-export line is an exact package pin with SHA-256 hashes.""" fields = re.split(r"\s+(?=--hash=)", line) @@ -590,34 +594,201 @@ def base_hash_locks(repo_root: pathlib.Path, base_sha: str) -> list[tuple[str, b return sorted(locks, key=lambda item: item[0]) +def _require_descriptor_relative_capabilities() -> None: + """Require the descriptor-relative primitives used for race-safe writes.""" + dir_fd_supported = getattr(os, "supports_dir_fd", set()) + fd_supported = getattr(os, "supports_fd", set()) + follow_symlinks_supported = getattr(os, "supports_follow_symlinks", set()) + if ( + any(function not in dir_fd_supported for function in _REQUIRED_DIR_FD_FUNCTIONS) + or any(function not in fd_supported for function in _REQUIRED_FD_FUNCTIONS) + or any( + function not in follow_symlinks_supported + for function in _REQUIRED_FOLLOW_SYMLINK_FUNCTIONS + ) + ): + raise ValueError("descriptor-relative output operations are unavailable") + if not all(hasattr(os, name) for name in ("O_DIRECTORY", "O_NOFOLLOW")): + raise ValueError("descriptor-relative output operations are unavailable") + + +def _reject_symlinked_output_components(output_dir: pathlib.Path) -> None: + """Reject symlinked ancestors before opening the materialization directory.""" + candidate = output_dir.absolute() + if candidate == pathlib.Path(candidate.anchor): + raise ValueError("output directory must not be the filesystem root") + current = pathlib.Path(candidate.anchor) + for component in candidate.parts[1:]: + current /= component + if current.is_symlink(): + raise ValueError( + "output directory must not be a symlink; " + f"path must not contain symlinks: {current}" + ) + if not current.exists(): + break + if not current.is_dir(): + raise ValueError( + f"output directory path component must be a directory: {current}" + ) + + +def _directory_identity(metadata: os.stat_result) -> tuple[int, int]: + """Return the device/inode identity of a regular directory.""" + if not stat.S_ISDIR(metadata.st_mode): + raise ValueError("output directory binding changed during secure materialization") + return metadata.st_dev, metadata.st_ino + + +def _verify_output_directory_binding( + output_dir: pathlib.Path, + output_fd: int, + identity: tuple[int, int], +) -> None: + """Fail closed if the published path no longer names the opened directory.""" + descriptor_metadata = os.fstat(output_fd) + try: + path_metadata = os.stat(output_dir.absolute(), follow_symlinks=False) + except OSError as exc: + raise ValueError("output directory changed during secure materialization") from exc + if ( + not stat.S_ISDIR(path_metadata.st_mode) + or (descriptor_metadata.st_dev, descriptor_metadata.st_ino) != identity + or (path_metadata.st_dev, path_metadata.st_ino) != identity + ): + raise ValueError("output directory changed during secure materialization") + + +def _open_relative_directory(root_fd: int, parts: tuple[str, ...]) -> int: + """Open or create each child directory without following symlinks.""" + current_fd = os.dup(root_fd) + try: + for part in parts: + created = False + try: + os.mkdir(part, mode=0o700, dir_fd=current_fd) + created = True + except FileExistsError: + pass + expected_identity = _directory_identity( + os.stat(part, dir_fd=current_fd, follow_symlinks=False) + ) + if created: + os.fsync(current_fd) + next_fd = os.open(part, _DIRECTORY_OPEN_FLAGS, dir_fd=current_fd) + try: + if _directory_identity(os.fstat(next_fd)) != expected_identity: + raise ValueError( + "output directory binding changed during secure materialization" + ) + os.fsync(next_fd) + except BaseException: + os.close(next_fd) + raise + os.close(current_fd) + current_fd = next_fd + return current_fd + except BaseException: + os.close(current_fd) + raise + + +def _open_output_directory(output_dir: pathlib.Path) -> tuple[int, tuple[int, int]]: + """Open a no-follow output directory and bind it to its observed inode.""" + candidate = output_dir.absolute() + _reject_symlinked_output_components(candidate) + anchor = pathlib.Path(candidate.anchor) + anchor_fd = os.open(anchor, _DIRECTORY_OPEN_FLAGS) + try: + output_fd = _open_relative_directory(anchor_fd, tuple(candidate.parts[1:])) + try: + os.fsync(output_fd) + metadata = os.fstat(output_fd) + identity = (metadata.st_dev, metadata.st_ino) + _verify_output_directory_binding(candidate, output_fd, identity) + return output_fd, identity + except BaseException: + os.close(output_fd) + raise + finally: + os.close(anchor_fd) + + +def _write_new_file(parent_fd: int, filename: str, content: bytes) -> None: + """Create and synchronize one singly-linked regular file by directory FD.""" + try: + file_fd = os.open(filename, _NEW_FILE_FLAGS, 0o600, dir_fd=parent_fd) + except FileExistsError as exc: + raise ValueError(f"generated output file must not pre-exist: {filename}") from exc + initial_metadata = os.fstat(file_fd) + identity = (initial_metadata.st_dev, initial_metadata.st_ino) + try: + if not stat.S_ISREG(initial_metadata.st_mode) or initial_metadata.st_nlink != 1: + raise ValueError("generated output files must be singly linked regular files") + view = memoryview(content) + offset = 0 + while offset < len(view): + written = os.write(file_fd, view[offset:]) + if written <= 0: + raise OSError("output write made no progress") + offset += written + os.fsync(file_fd) + final_metadata = os.fstat(file_fd) + path_metadata = os.stat(filename, dir_fd=parent_fd, follow_symlinks=False) + if ( + not stat.S_ISREG(path_metadata.st_mode) + or (final_metadata.st_dev, final_metadata.st_ino) + != (path_metadata.st_dev, path_metadata.st_ino) + or final_metadata.st_nlink != 1 + or path_metadata.st_nlink != 1 + ): + raise ValueError("output file changed during secure materialization") + os.fsync(parent_fd) + except BaseException: + try: + path_metadata = os.stat(filename, dir_fd=parent_fd, follow_symlinks=False) + if (path_metadata.st_dev, path_metadata.st_ino) == identity: + os.unlink(filename, dir_fd=parent_fd) + os.fsync(parent_fd) + except OSError: + pass + raise + finally: + os.close(file_fd) + + def materialize( repo_root: pathlib.Path, base_sha: str, output_dir: pathlib.Path, ) -> list[dict[str, str]]: """Write base lock blobs under generated names safe for a Docker build context.""" - if output_dir.exists() and output_dir.is_symlink(): - raise ValueError("output directory must not be a symlink") - output_dir.mkdir(parents=True, exist_ok=True) - - manifest: list[dict[str, str]] = [] - for index, (source_path, content) in enumerate( - base_hash_locks(repo_root.resolve(), base_sha) - ): - generated_name = f"requirements-{index:03d}.txt" - destination = output_dir / generated_name - destination.write_bytes(content) - manifest.append({"file": generated_name, "source": source_path}) - - (output_dir / "manifest.json").write_text( - json.dumps(manifest, indent=2, sort_keys=True) + "\n", - encoding="utf-8", - ) - (output_dir / "manifest.txt").write_text( - "".join(f"{entry['file']}\n" for entry in manifest), - encoding="utf-8", - ) - return manifest + _require_descriptor_relative_capabilities() + output_fd, output_identity = _open_output_directory(output_dir) + try: + manifest: list[dict[str, str]] = [] + for index, (source_path, content) in enumerate( + base_hash_locks(repo_root.resolve(), base_sha) + ): + generated_name = f"requirements-{index:03d}.txt" + _write_new_file(output_fd, generated_name, content) + manifest.append({"file": generated_name, "source": source_path}) + + _write_new_file( + output_fd, + "manifest.json", + (json.dumps(manifest, indent=2, sort_keys=True) + "\n").encode("utf-8"), + ) + _write_new_file( + output_fd, + "manifest.txt", + "".join(f"{entry['file']}\n" for entry in manifest).encode("utf-8"), + ) + os.fsync(output_fd) + _verify_output_directory_binding(output_dir, output_fd, output_identity) + return manifest + finally: + os.close(output_fd) def main(argv: list[str] | None = None) -> int: diff --git a/tests/test_javascript_materializer_creation_cleanup_security.py b/tests/test_javascript_materializer_creation_cleanup_security.py new file mode 100644 index 000000000..2ad7707b7 --- /dev/null +++ b/tests/test_javascript_materializer_creation_cleanup_security.py @@ -0,0 +1,152 @@ +"""Adversarial creation and rollback contracts for JavaScript lock materialization.""" + +from __future__ import annotations + +import os +from pathlib import Path + +import pytest + +from scripts.ci import materialize_base_javascript_packages as materializer + + +_BASE_SHA = "a" * 40 +_LOCK_BLOB_SHA = "b" * 40 + + +def _stub_projects( + monkeypatch: pytest.MonkeyPatch, + inputs: dict[str, bytes] | None = None, +) -> None: + """Replace Git discovery with one bounded npm project or an empty queue.""" + projects = [] + if inputs is not None: + projects = [("package-lock.json", "npm", inputs)] + monkeypatch.setattr( + materializer, + "base_npm_projects", + lambda *_args: projects, + ) + monkeypatch.setattr(materializer, "base_pnpm_projects", lambda *_args: []) + monkeypatch.setattr(materializer, "_lock_blob_sha", lambda *_args: _LOCK_BLOB_SHA) + + +def test_forwarding_open_instrumentation_does_not_change_platform_capability( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Capability checks use immutable CPython callables, not test wrappers.""" + output_directory = tmp_path / "generated_locks" + _stub_projects(monkeypatch) + real_open = os.open + + def forwarding_open( + path: object, + flags: int, + *args: object, + **kwargs: object, + ) -> int: + return real_open(path, flags, *args, **kwargs) + + monkeypatch.setattr(os, "open", forwarding_open) + + manifest = materializer.materialize(tmp_path, _BASE_SHA, output_directory) + + assert manifest == [] + assert (output_directory / "manifest.json").read_text(encoding="utf-8") == "[]\n" + + +def test_materializer_rejects_missing_follow_symlink_capability_before_mutation( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """No-follow stat support is mandatory before any output path is created.""" + output_directory = tmp_path / "generated_locks" + _stub_projects(monkeypatch) + monkeypatch.setattr(os, "supports_follow_symlinks", set()) + + with pytest.raises(ValueError, match="descriptor-relative.*unavailable"): + materializer.materialize(tmp_path, _BASE_SHA, output_directory) + + assert not output_directory.exists() + + +def test_missing_ancestor_swap_never_creates_output_through_attacker_symlink( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Pathname creation cannot be redirected while an ancestor is replaced.""" + trusted_root = tmp_path / "trusted_root" + trusted_root.mkdir() + pinned_root = tmp_path / "pinned_root" + attacker_root = tmp_path / "attacker_root" + attacker_parent = attacker_root / "missing_parent" + attacker_parent.mkdir(parents=True) + output_directory = trusted_root / "missing_parent" / "generated_locks" + attacker_output = attacker_parent / "generated_locks" + _stub_projects(monkeypatch) + + real_mkdir = os.mkdir + swapped = False + + def swap_after_parent_creation( + path: object, + *args: object, + **kwargs: object, + ) -> None: + nonlocal swapped + real_mkdir(path, *args, **kwargs) + if ( + not swapped + and path == output_directory.parent.name + and kwargs.get("dir_fd") is not None + ): + trusted_root.rename(pinned_root) + trusted_root.symlink_to(attacker_root, target_is_directory=True) + swapped = True + + monkeypatch.setattr(os, "mkdir", swap_after_parent_creation) + + with pytest.raises(ValueError, match="ancestor|symlink|changed"): + materializer.materialize(tmp_path, _BASE_SHA, output_directory) + + assert swapped is True + assert not attacker_output.exists() + + +def test_late_write_failure_rolls_back_every_owned_file_and_directory( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Rollback removes all earlier generated entries while preserving operator data.""" + output_directory = tmp_path / "generated_locks" + output_directory.mkdir() + operator_note = output_directory / "operator-note.txt" + operator_note.write_text("preserve\n", encoding="utf-8") + _stub_projects( + monkeypatch, + { + "a-first.json": b"first\n", + "b-second.json": b"second\n", + }, + ) + real_write = os.write + write_calls = 0 + + def fail_second_file_write(file_descriptor: int, content: object) -> int: + nonlocal write_calls + write_calls += 1 + if write_calls == 2: + return 0 + return real_write(file_descriptor, content) + + monkeypatch.setattr(os, "write", fail_second_file_write) + + with pytest.raises(OSError, match="made no progress"): + materializer.materialize(tmp_path, _BASE_SHA, output_directory) + + assert write_calls == 2 + assert operator_note.read_text(encoding="utf-8") == "preserve\n" + assert sorted(path.name for path in output_directory.iterdir()) == [ + "operator-note.txt" + ] diff --git a/tests/test_javascript_materializer_descriptor_ancestry.py b/tests/test_javascript_materializer_descriptor_ancestry.py new file mode 100644 index 000000000..18872624f --- /dev/null +++ b/tests/test_javascript_materializer_descriptor_ancestry.py @@ -0,0 +1,189 @@ +"""Adversarial contracts for descriptor-anchored materializer ancestry and cleanup.""" + +from __future__ import annotations + +import os +from pathlib import Path +import stat + +import pytest + +from scripts.ci import materialize_base_javascript_packages as materializer + + +_BASE_SHA = "a" * 40 +_LOCK_BLOB_SHA = "b" * 40 + + +def _projects(relative_path: str = "package-lock.json") -> list[tuple[str, str, dict[str, bytes]]]: + """Return one deterministic npm project with one optionally nested lock input.""" + return [ + ( + "package-lock.json", + "npm", + { + "package.json": b'{"name":"fixture"}\n', + relative_path: b'{"lockfileVersion":3,"packages":{}}\n', + }, + ) + ] + + +def _stub_project_discovery( + monkeypatch: pytest.MonkeyPatch, + projects: list[tuple[str, str, dict[str, bytes]]] | None = None, +) -> None: + """Replace Git-backed project discovery with bounded in-memory fixtures.""" + monkeypatch.setattr( + materializer, + "base_npm_projects", + lambda *_args: _projects() if projects is None else projects, + ) + monkeypatch.setattr(materializer, "base_pnpm_projects", lambda *_args: []) + monkeypatch.setattr(materializer, "_lock_blob_sha", lambda *_args: _LOCK_BLOB_SHA) + + +def test_materializer_rejects_intermediate_ancestor_swap_before_parent_open( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An intermediate ancestor swap cannot redirect the initially opened output tree.""" + trusted_root = tmp_path / "trusted_root" + trusted_parent = trusted_root / "nested_parent" + output_directory = trusted_parent / "generated_locks" + trusted_parent.mkdir(parents=True) + + pinned_root = tmp_path / "pinned_root" + attacker_root = tmp_path / "attacker_root" + attacker_output = attacker_root / "nested_parent" / "generated_locks" + attacker_output.mkdir(parents=True) + _stub_project_discovery(monkeypatch) + + real_open = os.open + swapped = False + + def swap_intermediate_ancestor( + path: object, + flags: int, + *args: object, + **kwargs: object, + ) -> int: + nonlocal swapped + if ( + not swapped + and path == trusted_parent.name + and kwargs.get("dir_fd") is not None + ): + trusted_root.rename(pinned_root) + trusted_root.symlink_to(attacker_root, target_is_directory=True) + swapped = True + return real_open(path, flags, *args, **kwargs) + + monkeypatch.setattr(os, "open", swap_intermediate_ancestor) + + with pytest.raises(ValueError, match="ancestor|symlink|changed"): + materializer.materialize(tmp_path, _BASE_SHA, output_directory) + + assert swapped is True + assert list(attacker_output.iterdir()) == [] + assert list((pinned_root / "nested_parent" / "generated_locks").iterdir()) == [] + + +def test_materializer_rejects_new_nested_directory_replacement_before_open( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A directory created beneath a held descriptor must retain its original inode.""" + output_directory = tmp_path / "generated_locks" + saved_directory = tmp_path / "saved_nested_directory" + _stub_project_discovery(monkeypatch, _projects("nested_directory/package-lock.json")) + + real_open = os.open + swapped = False + + def swap_nested_directory( + path: object, + flags: int, + *args: object, + **kwargs: object, + ) -> int: + nonlocal swapped + if ( + not swapped + and path == "nested_directory" + and kwargs.get("dir_fd") is not None + ): + nested_directory = output_directory / "project-000" / "nested_directory" + nested_directory.rename(saved_directory) + nested_directory.mkdir() + swapped = True + return real_open(path, flags, *args, **kwargs) + + monkeypatch.setattr(os, "open", swap_nested_directory) + + with pytest.raises(ValueError, match="directory.*changed|binding|inode"): + materializer.materialize(tmp_path, _BASE_SHA, output_directory) + + assert swapped is True + assert list(saved_directory.iterdir()) == [] + replacement = output_directory / "project-000" / "nested_directory" + assert not (replacement / "package-lock.json").exists() + + +def test_materializer_fsyncs_files_and_every_published_directory( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Durable evidence requires file bytes and directory entries to be synchronized.""" + output_directory = tmp_path / "generated_locks" + _stub_project_discovery(monkeypatch, _projects("nested_directory/package-lock.json")) + real_fsync = os.fsync + synchronized_modes: list[int] = [] + + def track_fsync(file_descriptor: int) -> None: + synchronized_modes.append(stat.S_IFMT(os.fstat(file_descriptor).st_mode)) + real_fsync(file_descriptor) + + monkeypatch.setattr(os, "fsync", track_fsync) + + materializer.materialize(tmp_path, _BASE_SHA, output_directory) + + assert stat.S_IFREG in synchronized_modes + assert stat.S_IFDIR in synchronized_modes + assert synchronized_modes.count(stat.S_IFDIR) >= 3 + + +def test_materializer_fails_closed_without_descriptor_relative_capabilities( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Unsupported runtimes must fail before creating any output path.""" + output_directory = tmp_path / "generated_locks" + _stub_project_discovery(monkeypatch, []) + monkeypatch.setattr(os, "supports_dir_fd", set()) + + with pytest.raises(ValueError, match="descriptor-relative.*unavailable"): + materializer.materialize(tmp_path, _BASE_SHA, output_directory) + + assert not output_directory.exists() + + +def test_failed_write_removes_only_owned_outputs_and_preserves_existing_entries( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Failure cleanup removes partial generated evidence without deleting prior content.""" + output_directory = tmp_path / "generated_locks" + output_directory.mkdir() + existing_file = output_directory / "operator-note.txt" + existing_file.write_text("preserve\n", encoding="utf-8") + _stub_project_discovery(monkeypatch) + monkeypatch.setattr(os, "write", lambda *_args: 0) + + with pytest.raises(OSError, match="made no progress"): + materializer.materialize(tmp_path, _BASE_SHA, output_directory) + + assert existing_file.read_text(encoding="utf-8") == "preserve\n" + assert sorted(path.name for path in output_directory.iterdir()) == [ + "operator-note.txt" + ] diff --git a/tests/test_javascript_materializer_existing_directory_coverage.py b/tests/test_javascript_materializer_existing_directory_coverage.py new file mode 100644 index 000000000..71e8df5c1 --- /dev/null +++ b/tests/test_javascript_materializer_existing_directory_coverage.py @@ -0,0 +1,25 @@ +"""Existing-directory branch coverage for the JavaScript lock materializer.""" + +from __future__ import annotations + +import os +from pathlib import Path + +from scripts.ci import materialize_base_javascript_packages as materializer + + +def test_relative_directory_reuses_existing_directory(tmp_path: Path) -> None: + """A pre-existing nested directory is opened without the creation-only sync path.""" + + nested_directory = tmp_path / "nested_directory" + nested_directory.mkdir() + root_fd = os.open(tmp_path, materializer._DIRECTORY_OPEN_FLAGS) + nested_fd = materializer._open_relative_directory(root_fd, (nested_directory.name,)) + try: + assert os.path.samestat( + os.fstat(nested_fd), + os.stat(nested_directory, follow_symlinks=False), + ) + finally: + os.close(nested_fd) + os.close(root_fd) diff --git a/tests/test_javascript_materializer_output_edge_coverage.py b/tests/test_javascript_materializer_output_edge_coverage.py new file mode 100644 index 000000000..e249d57a9 --- /dev/null +++ b/tests/test_javascript_materializer_output_edge_coverage.py @@ -0,0 +1,390 @@ +"""Branch-complete edge contracts for JavaScript materializer output hardening.""" + +from __future__ import annotations + +import errno +import os +from pathlib import Path + +import pytest + +from scripts.ci import materialize_base_javascript_packages as materializer + + +def _different_inode(metadata: os.stat_result) -> os.stat_result: + """Return metadata with the inode changed while retaining all other fields.""" + + values = list(metadata) + values[1] = metadata.st_ino + 1 + return os.stat_result(values) + + +def test_capability_gate_rejects_missing_no_follow_flag( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Secure output publication fails when a required open flag is unavailable.""" + + monkeypatch.delattr(materializer.os, "O_NOFOLLOW") + + with pytest.raises(ValueError, match="descriptor-relative output operations"): + materializer._require_descriptor_relative_capabilities() + + +def test_component_scan_rejects_existing_regular_file(tmp_path: Path) -> None: + """A regular file cannot become an intermediate output-directory component.""" + + blocking_file = tmp_path / "blocking_file" + blocking_file.write_bytes(b"not a directory") + + with pytest.raises(ValueError, match="path component must be a directory"): + materializer._reject_symlinked_output_components( + blocking_file / "generated_locks" + ) + + +def test_directory_identity_rejects_non_directory_metadata(tmp_path: Path) -> None: + """Directory identities reject regular-file metadata before inode comparison.""" + + regular_file = tmp_path / "regular_file" + regular_file.write_bytes(b"content") + + with pytest.raises(ValueError, match="binding changed"): + materializer._directory_identity(regular_file.stat()) + + +def test_output_open_detects_parent_descriptor_replacement( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The opened parent descriptor must retain the pre-open parent identity.""" + + output_directory = tmp_path / "generated_locks" + expected_parent = output_directory.parent.name + real_open = materializer.os.open + real_fstat = materializer.os.fstat + parent_descriptors: list[int] = [] + + def capture_parent_open( + path: object, + flags: int, + *args: object, + **kwargs: object, + ) -> int: + descriptor = real_open(path, flags, *args, **kwargs) + if path == expected_parent and kwargs.get("dir_fd") is not None: + parent_descriptors.append(descriptor) + return descriptor + + def replace_parent_identity(descriptor: int) -> os.stat_result: + metadata = real_fstat(descriptor) + if descriptor in parent_descriptors: + return _different_inode(metadata) + return metadata + + monkeypatch.setattr(materializer.os, "open", capture_parent_open) + monkeypatch.setattr(materializer.os, "fstat", replace_parent_identity) + + with pytest.raises(ValueError, match="binding changed"): + materializer._open_output_directory(output_directory) + + assert len(parent_descriptors) == 1 + with pytest.raises(OSError) as raised: + os.fstat(parent_descriptors[0]) + assert raised.value.errno == errno.EBADF + + +def test_output_open_detects_output_descriptor_replacement_and_closes_it( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The opened output descriptor is closed when its inode mismatches the path.""" + + output_directory = tmp_path / "generated_locks" + real_open = materializer.os.open + real_fstat = materializer.os.fstat + output_descriptors: list[int] = [] + + def capture_output_open( + path: object, + flags: int, + *args: object, + **kwargs: object, + ) -> int: + descriptor = real_open(path, flags, *args, **kwargs) + if path == output_directory.name and kwargs.get("dir_fd") is not None: + output_descriptors.append(descriptor) + return descriptor + + def replace_output_identity(descriptor: int) -> os.stat_result: + metadata = real_fstat(descriptor) + if descriptor in output_descriptors: + return _different_inode(metadata) + return metadata + + monkeypatch.setattr(materializer.os, "open", capture_output_open) + monkeypatch.setattr(materializer.os, "fstat", replace_output_identity) + + with pytest.raises(ValueError, match="output directory.*changed"): + materializer._open_output_directory(output_directory) + + assert len(output_descriptors) == 1 + with pytest.raises(OSError) as raised: + os.fstat(output_descriptors[0]) + assert raised.value.errno == errno.EBADF + + +def test_relative_directory_creation_synchronizes_new_directory(tmp_path: Path) -> None: + """A newly created nested directory returns a live pinned descriptor.""" + + root_fd = os.open(tmp_path, materializer._DIRECTORY_OPEN_FLAGS) + nested_fd = materializer._open_relative_directory(root_fd, ("nested_directory",)) + try: + assert (tmp_path / "nested_directory").is_dir() + assert os.path.samestat( + os.fstat(nested_fd), + os.stat(tmp_path / "nested_directory", follow_symlinks=False), + ) + finally: + os.close(nested_fd) + os.close(root_fd) + + +def test_relative_directory_detects_descriptor_replacement_and_closes_it( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A child descriptor is closed when it differs from the pre-open child inode.""" + + root_fd = os.open(tmp_path, materializer._DIRECTORY_OPEN_FLAGS) + real_open = materializer.os.open + real_fstat = materializer.os.fstat + child_descriptors: list[int] = [] + + def capture_child_open( + path: object, + flags: int, + *args: object, + **kwargs: object, + ) -> int: + descriptor = real_open(path, flags, *args, **kwargs) + if path == "nested_directory" and kwargs.get("dir_fd") is not None: + child_descriptors.append(descriptor) + return descriptor + + def replace_child_identity(descriptor: int) -> os.stat_result: + metadata = real_fstat(descriptor) + if descriptor in child_descriptors: + return _different_inode(metadata) + return metadata + + monkeypatch.setattr(materializer.os, "open", capture_child_open) + monkeypatch.setattr(materializer.os, "fstat", replace_child_identity) + try: + with pytest.raises(ValueError, match="binding changed"): + materializer._open_relative_directory(root_fd, ("nested_directory",)) + finally: + os.close(root_fd) + + assert len(child_descriptors) == 1 + with pytest.raises(OSError) as raised: + os.fstat(child_descriptors[0]) + assert raised.value.errno == errno.EBADF + + +def test_project_directory_detects_descriptor_replacement_and_closes_it( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A fresh project descriptor is closed when its inode fails revalidation.""" + + output_fd = os.open(tmp_path, materializer._DIRECTORY_OPEN_FLAGS) + real_open = materializer.os.open + real_fstat = materializer.os.fstat + project_descriptors: list[int] = [] + + def capture_project_open( + path: object, + flags: int, + *args: object, + **kwargs: object, + ) -> int: + descriptor = real_open(path, flags, *args, **kwargs) + if path == "project-000" and kwargs.get("dir_fd") == output_fd: + project_descriptors.append(descriptor) + return descriptor + + def replace_project_identity(descriptor: int) -> os.stat_result: + metadata = real_fstat(descriptor) + if descriptor in project_descriptors: + return _different_inode(metadata) + return metadata + + monkeypatch.setattr(materializer.os, "open", capture_project_open) + monkeypatch.setattr(materializer.os, "fstat", replace_project_identity) + try: + with pytest.raises(ValueError, match="binding changed"): + materializer._create_project_directory(output_fd, "project-000") + finally: + os.close(output_fd) + + assert len(project_descriptors) == 1 + with pytest.raises(OSError) as raised: + os.fstat(project_descriptors[0]) + assert raised.value.errno == errno.EBADF + + +def test_unlink_owned_file_ignores_missing_name(tmp_path: Path) -> None: + """Cleanup is a no-op when the generated filename no longer exists.""" + + parent_fd = os.open(tmp_path, materializer._DIRECTORY_OPEN_FLAGS) + try: + materializer._unlink_owned_file(parent_fd, "missing_file", (1, 1)) + finally: + os.close(parent_fd) + + +def test_unlink_owned_file_ignores_replaced_identity(tmp_path: Path) -> None: + """Cleanup never unlinks a path that no longer names the generated inode.""" + + destination = tmp_path / "generated_file" + destination.write_bytes(b"replacement") + parent_fd = os.open(tmp_path, materializer._DIRECTORY_OPEN_FLAGS) + try: + materializer._unlink_owned_file(parent_fd, destination.name, (1, 1)) + finally: + os.close(parent_fd) + assert destination.read_bytes() == b"replacement" + + +def test_unlink_owned_file_ignores_unlink_failure( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Cleanup remains fail-safe when the owned filename cannot be unlinked.""" + + destination = tmp_path / "generated_file" + destination.write_bytes(b"content") + metadata = destination.stat() + parent_fd = os.open(tmp_path, materializer._DIRECTORY_OPEN_FLAGS) + + def deny_unlink(*_args: object, **_kwargs: object) -> None: + raise PermissionError(errno.EACCES, "synthetic") + + monkeypatch.setattr(materializer.os, "unlink", deny_unlink) + try: + materializer._unlink_owned_file( + parent_fd, + destination.name, + (metadata.st_dev, metadata.st_ino), + ) + finally: + os.close(parent_fd) + assert destination.read_bytes() == b"content" + + +def test_remove_owned_directory_ignores_missing_name(tmp_path: Path) -> None: + """Directory cleanup is a no-op when the generated directory disappeared.""" + + parent_fd = os.open(tmp_path, materializer._DIRECTORY_OPEN_FLAGS) + try: + materializer._remove_owned_empty_directory( + parent_fd, + "missing_directory", + (1, 1), + ) + finally: + os.close(parent_fd) + + +def test_remove_owned_directory_ignores_regular_file(tmp_path: Path) -> None: + """Directory cleanup never removes a regular file at the generated name.""" + + destination = tmp_path / "project-000" + destination.write_bytes(b"content") + metadata = destination.stat() + parent_fd = os.open(tmp_path, materializer._DIRECTORY_OPEN_FLAGS) + try: + materializer._remove_owned_empty_directory( + parent_fd, + destination.name, + (metadata.st_dev, metadata.st_ino), + ) + finally: + os.close(parent_fd) + assert destination.read_bytes() == b"content" + + +def test_remove_owned_directory_ignores_replaced_identity(tmp_path: Path) -> None: + """Directory cleanup preserves a directory whose inode no longer matches.""" + + destination = tmp_path / "project-000" + destination.mkdir() + parent_fd = os.open(tmp_path, materializer._DIRECTORY_OPEN_FLAGS) + try: + materializer._remove_owned_empty_directory( + parent_fd, + destination.name, + (1, 1), + ) + finally: + os.close(parent_fd) + assert destination.is_dir() + + +def test_remove_owned_directory_ignores_rmdir_failure(tmp_path: Path) -> None: + """Nonempty owned directories remain available for forensic inspection.""" + + destination = tmp_path / "project-000" + destination.mkdir() + (destination / "retained_file").write_bytes(b"content") + metadata = destination.stat() + parent_fd = os.open(tmp_path, materializer._DIRECTORY_OPEN_FLAGS) + try: + materializer._remove_owned_empty_directory( + parent_fd, + destination.name, + (metadata.st_dev, metadata.st_ino), + ) + finally: + os.close(parent_fd) + assert (destination / "retained_file").read_bytes() == b"content" + + +def test_owned_cleanup_rejects_replaced_child_descriptor( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Recursive cleanup never descends through a replaced child binding.""" + + project_directory = tmp_path / "project-000" + child_directory = project_directory / "nested_directory" + child_directory.mkdir(parents=True) + project_fd = os.open(project_directory, materializer._DIRECTORY_OPEN_FLAGS) + real_open = materializer.os.open + real_fstat = materializer.os.fstat + child_descriptors: list[int] = [] + + def capture_child_open( + path: object, + flags: int, + *args: object, + **kwargs: object, + ) -> int: + descriptor = real_open(path, flags, *args, **kwargs) + if path == child_directory.name and kwargs.get("dir_fd") == project_fd: + child_descriptors.append(descriptor) + return descriptor + + def replace_child_identity(descriptor: int) -> os.stat_result: + metadata = real_fstat(descriptor) + if descriptor in child_descriptors: + return _different_inode(metadata) + return metadata + + monkeypatch.setattr(materializer.os, "open", capture_child_open) + monkeypatch.setattr(materializer.os, "fstat", replace_child_identity) + try: + with pytest.raises(ValueError, match="binding changed"): + materializer._remove_owned_directory_contents(project_fd) + finally: + os.close(project_fd) + + assert child_directory.is_dir() + assert len(child_descriptors) == 1 + with pytest.raises(OSError) as raised: + os.fstat(child_descriptors[0]) + assert raised.value.errno == errno.EBADF diff --git a/tests/test_javascript_materializer_output_security.py b/tests/test_javascript_materializer_output_security.py new file mode 100644 index 000000000..06e70809c --- /dev/null +++ b/tests/test_javascript_materializer_output_security.py @@ -0,0 +1,387 @@ +"""Security regressions for descriptor-pinned JavaScript lock materialization.""" + +from __future__ import annotations + +import errno +import os +from pathlib import Path + +import pytest + +from scripts.ci import materialize_base_javascript_packages as materializer + + +def _one_project(relative_path: str = "package-lock.json") -> list[tuple[str, str, dict[str, bytes]]]: + """Return one deterministic trusted npm project fixture.""" + + return [ + ( + "package-lock.json", + "npm", + { + "package.json": b'{"name":"fixture"}\n', + relative_path: b'{"lockfileVersion":3,"packages":{}}\n', + }, + ) + ] + + +def _stub_project_discovery( + monkeypatch: pytest.MonkeyPatch, + projects: list[tuple[str, str, dict[str, bytes]]] | None = None, +) -> None: + """Replace Git-backed discovery with one bounded in-memory project.""" + + monkeypatch.setattr( + materializer, + "base_npm_projects", + lambda *_args: _one_project() if projects is None else projects, + ) + monkeypatch.setattr(materializer, "base_pnpm_projects", lambda *_args: []) + monkeypatch.setattr(materializer, "_lock_blob_sha", lambda *_args: "b" * 40) + + +def test_materializer_rejects_symlinked_output_parent( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """No intermediate symlink may redirect descriptor-relative output creation.""" + + target_directory = tmp_path / "target_directory" + target_directory.mkdir() + linked_parent = tmp_path / "linked_parent" + linked_parent.symlink_to(target_directory, target_is_directory=True) + _stub_project_discovery(monkeypatch, []) + + with pytest.raises(ValueError, match="must not contain symlinks"): + materializer.materialize( + tmp_path, + "a" * 40, + linked_parent / "generated_locks", + ) + + assert list(target_directory.iterdir()) == [] + + +def test_materializer_fails_closed_when_output_binding_is_replaced( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Replacing the published pathname cannot receive trusted lock inputs.""" + + output_directory = tmp_path / "generated_locks" + pinned_directory = tmp_path / "pinned_locks" + replacement_directory = tmp_path / "replacement_locks" + + def replace_output_before_return( + *_args: object, + ) -> list[tuple[str, str, dict[str, bytes]]]: + output_directory.rename(pinned_directory) + replacement_directory.mkdir() + replacement_directory.rename(output_directory) + return _one_project() + + monkeypatch.setattr( + materializer, + "base_npm_projects", + replace_output_before_return, + ) + monkeypatch.setattr(materializer, "base_pnpm_projects", lambda *_args: []) + monkeypatch.setattr(materializer, "_lock_blob_sha", lambda *_args: "b" * 40) + + with pytest.raises(ValueError, match="changed during secure materialization"): + materializer.materialize(tmp_path, "a" * 40, output_directory) + + assert (pinned_directory / "project-000" / "package-lock.json").is_file() + assert list(output_directory.iterdir()) == [] + + +def test_materializer_anchors_writes_when_output_path_becomes_symlink( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A post-open output symlink cannot redirect the first generated file.""" + + output_directory = tmp_path / "generated_locks" + pinned_directory = tmp_path / "pinned_locks" + attacker_directory = tmp_path / "attacker_directory" + attacker_directory.mkdir() + _stub_project_discovery(monkeypatch) + real_open = os.open + attacked = False + + def swap_before_first_file_open( + path: object, + flags: int, + *args: object, + **kwargs: object, + ) -> int: + nonlocal attacked + if not attacked and path == "package-lock.json" and flags & os.O_CREAT: + attacked = True + output_directory.rename(pinned_directory) + output_directory.symlink_to(attacker_directory, target_is_directory=True) + return real_open(path, flags, *args, **kwargs) + + monkeypatch.setattr(os, "open", swap_before_first_file_open) + + with pytest.raises(ValueError, match="changed during secure materialization"): + materializer.materialize(tmp_path, "a" * 40, output_directory) + + assert attacked is True + assert ( + pinned_directory / "project-000" / "package-lock.json" + ).read_bytes() == _one_project()[0][2]["package-lock.json"] + assert list(attacker_directory.iterdir()) == [] + + +@pytest.mark.parametrize("relative_path", ["", "../escape", "/absolute", "nested\\escape"]) +def test_materializer_rejects_unsafe_relative_input_paths( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + relative_path: str, +) -> None: + """Trusted inputs still require one lexical relative POSIX output path.""" + + _stub_project_discovery(monkeypatch, _one_project(relative_path)) + + with pytest.raises(ValueError, match="unsafe relative output path"): + materializer.materialize( + tmp_path, + "a" * 40, + tmp_path / "generated_locks", + ) + + assert not (tmp_path / "escape").exists() + + +def test_materializer_rejects_preexisting_generated_file( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A pre-existing generated name cannot be truncated or reinterpreted.""" + + output_directory = tmp_path / "generated_locks" + project_directory = output_directory / "project-000" + project_directory.mkdir(parents=True) + destination = project_directory / "package-lock.json" + destination.write_bytes(b"unchanged") + _stub_project_discovery(monkeypatch) + + with pytest.raises(ValueError, match="must not pre-exist"): + materializer.materialize(tmp_path, "a" * 40, output_directory) + + assert destination.read_bytes() == b"unchanged" + + +def test_materializer_detects_hard_link_added_during_pinned_write( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A hard link added after file creation must fail before success evidence.""" + + output_directory = tmp_path / "generated_locks" + outside_link = tmp_path / "captured_output" + _stub_project_discovery(monkeypatch) + real_fsync = os.fsync + linked = False + + def link_after_file_sync(file_descriptor: int) -> None: + nonlocal linked + real_fsync(file_descriptor) + destination = output_directory / "project-000" / "package-lock.json" + if linked or not destination.exists(): + return + descriptor_metadata = os.fstat(file_descriptor) + path_metadata = os.stat(destination, follow_symlinks=False) + if (descriptor_metadata.st_dev, descriptor_metadata.st_ino) != ( + path_metadata.st_dev, + path_metadata.st_ino, + ): + return + os.link(destination, outside_link) + linked = True + + monkeypatch.setattr(os, "fsync", link_after_file_sync) + + with pytest.raises(ValueError, match="singly linked regular files"): + materializer.materialize(tmp_path, "a" * 40, output_directory) + + assert linked is True + assert outside_link.read_bytes() == _one_project()[0][2]["package-lock.json"] + + +def test_materializer_detects_destination_swap_after_pinned_write( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A generated name swapped after open cannot become accepted evidence.""" + + output_directory = tmp_path / "generated_locks" + outside_file = tmp_path / "outside_file" + outside_file.write_bytes(b"unchanged") + _stub_project_discovery(monkeypatch) + real_fsync = os.fsync + swapped = False + + def swap_after_file_sync(file_descriptor: int) -> None: + nonlocal swapped + real_fsync(file_descriptor) + destination = output_directory / "project-000" / "package-lock.json" + if swapped or not destination.exists(): + return + swapped = True + destination.unlink() + destination.symlink_to(outside_file) + + monkeypatch.setattr(os, "fsync", swap_after_file_sync) + + with pytest.raises(ValueError, match="output file changed"): + materializer.materialize(tmp_path, "a" * 40, output_directory) + + assert outside_file.read_bytes() == b"unchanged" + + +def test_materializer_fails_when_descriptor_write_makes_no_progress( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A zero-length descriptor write is an error, not truncated success.""" + + _stub_project_discovery(monkeypatch) + monkeypatch.setattr(os, "write", lambda *_args: 0) + + with pytest.raises(OSError, match="made no progress"): + materializer.materialize( + tmp_path, + "a" * 40, + tmp_path / "generated_locks", + ) + + +def test_materializer_rejects_filesystem_root_output( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The filesystem root is never a generated-lock output directory.""" + + _stub_project_discovery(monkeypatch, []) + + with pytest.raises(ValueError, match="must not be the filesystem root"): + materializer.materialize(tmp_path, "a" * 40, Path("/")) + + +def test_materializer_preserves_bounded_directory_open_failures( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Descriptor-relative ENOTDIR remains fail-closed without a full-path leak.""" + + _stub_project_discovery(monkeypatch, []) + real_open = os.open + + def fail_output_open( + path: object, + flags: int, + *args: object, + **kwargs: object, + ) -> int: + if path == "generated_locks": + raise OSError(errno.ENOTDIR, "synthetic") + return real_open(path, flags, *args, **kwargs) + + monkeypatch.setattr(os, "open", fail_output_open) + + with pytest.raises(NotADirectoryError, match="synthetic") as raised: + materializer.materialize( + tmp_path, + "a" * 40, + tmp_path / "generated_locks", + ) + assert raised.value.errno == errno.ENOTDIR + assert raised.value.filename is None + + +def test_output_binding_rejects_removed_published_path(tmp_path: Path) -> None: + """A removed output pathname cannot validate against its still-open descriptor.""" + + output_directory = tmp_path / "generated_locks" + output_directory.mkdir() + output_fd = os.open(output_directory, materializer._DIRECTORY_OPEN_FLAGS) + metadata = os.fstat(output_fd) + try: + output_directory.rmdir() + with pytest.raises(ValueError, match="changed during secure materialization"): + materializer._verify_output_directory_binding( + output_directory, + output_fd, + (metadata.st_dev, metadata.st_ino), + ) + finally: + os.close(output_fd) + + +def test_relative_directory_open_failure_closes_fail_closed( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A child directory that cannot be opened propagates a bounded hard failure.""" + + root_fd = os.open(tmp_path, materializer._DIRECTORY_OPEN_FLAGS) + real_open = os.open + + def fail_child_open( + path: object, + flags: int, + *args: object, + **kwargs: object, + ) -> int: + if path == "nested_directory": + raise OSError(errno.EACCES, "synthetic") + return real_open(path, flags, *args, **kwargs) + + monkeypatch.setattr(os, "open", fail_child_open) + try: + with pytest.raises(OSError, match="synthetic"): + materializer._open_relative_directory(root_fd, ("nested_directory",)) + finally: + os.close(root_fd) + + +def test_project_directory_must_be_fresh(tmp_path: Path) -> None: + """A pre-existing numbered project directory is rejected before any file write.""" + + (tmp_path / "project-000").mkdir() + output_fd = os.open(tmp_path, materializer._DIRECTORY_OPEN_FLAGS) + try: + with pytest.raises(ValueError, match="must not pre-exist"): + materializer._create_project_directory(output_fd, "project-000") + finally: + os.close(output_fd) + + +def test_descriptor_file_must_be_fresh(tmp_path: Path) -> None: + """A pre-existing file name cannot be reopened through the descriptor helper.""" + + (tmp_path / "manifest.json").write_bytes(b"unchanged") + parent_fd = os.open(tmp_path, materializer._DIRECTORY_OPEN_FLAGS) + try: + with pytest.raises(ValueError, match="must not pre-exist"): + materializer._write_new_file(parent_fd, "manifest.json", b"replacement") + finally: + os.close(parent_fd) + assert (tmp_path / "manifest.json").read_bytes() == b"unchanged" + + +def test_new_file_rejects_non_single_link_initial_descriptor( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """An unexpected initial link count fails before trusted bytes are written.""" + + parent_fd = os.open(tmp_path, materializer._DIRECTORY_OPEN_FLAGS) + real_fstat = os.fstat + + def force_multiple_links(file_descriptor: int) -> os.stat_result: + metadata = real_fstat(file_descriptor) + if file_descriptor == parent_fd: + return metadata + values = list(metadata) + values[3] = 2 + return os.stat_result(values) + + monkeypatch.setattr(os, "fstat", force_multiple_links) + try: + with pytest.raises(ValueError, match="singly linked regular files"): + materializer._write_new_file(parent_fd, "new-lock.json", b"trusted") + finally: + os.close(parent_fd) diff --git a/tests/test_materialize_base_python_requirements.py b/tests/test_materialize_base_python_requirements.py index 5bc56ed8f..a7c252def 100644 --- a/tests/test_materialize_base_python_requirements.py +++ b/tests/test_materialize_base_python_requirements.py @@ -2,6 +2,7 @@ import hashlib import io +import os import runpy import subprocess import sys @@ -236,6 +237,184 @@ def test_rejects_symlink_output_directory( materializer.materialize(tmp_path, "a" * 40, output) +def test_descriptor_operations_fail_closed_when_unavailable( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Materialization refuses platforms without descriptor-relative primitives.""" + monkeypatch.setattr(materializer.os, "supports_dir_fd", set()) + with pytest.raises(ValueError, match="descriptor-relative"): + materializer._require_descriptor_relative_capabilities() + + monkeypatch.setattr( + materializer.os, + "supports_dir_fd", + set(materializer._REQUIRED_DIR_FD_FUNCTIONS), + ) + monkeypatch.setattr(materializer.os, "supports_fd", set()) + with pytest.raises(ValueError, match="descriptor-relative"): + materializer._require_descriptor_relative_capabilities() + + monkeypatch.setattr( + materializer.os, + "supports_fd", + set(materializer._REQUIRED_FD_FUNCTIONS), + ) + monkeypatch.setattr(materializer.os, "supports_follow_symlinks", set()) + with pytest.raises(ValueError, match="descriptor-relative"): + materializer._require_descriptor_relative_capabilities() + + +def test_descriptor_operations_require_no_follow_flags( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Materialization refuses an operating system without no-follow flags.""" + monkeypatch.delattr(materializer.os, "O_DIRECTORY") + with pytest.raises(ValueError, match="descriptor-relative"): + materializer._require_descriptor_relative_capabilities() + + +def test_output_path_components_and_directory_identity_fail_closed( + tmp_path: Path, +) -> None: + """Root, file, and non-directory identities are never accepted as outputs.""" + with pytest.raises(ValueError, match="filesystem root"): + materializer._reject_symlinked_output_components(Path("/")) + + file_path = tmp_path / "not-a-directory" + file_path.write_text("x", encoding="utf-8") + with pytest.raises(ValueError, match="must be a directory"): + materializer._reject_symlinked_output_components(file_path) + with pytest.raises(ValueError, match="changed"): + materializer._directory_identity(file_path.stat()) + + +def test_output_binding_detects_removed_or_rebound_directory( + tmp_path: Path, +) -> None: + """The opened descriptor must continue to name the published output path.""" + output = tmp_path / "output" + output.mkdir() + output_fd, identity = materializer._open_output_directory(output) + try: + output.rmdir() + with pytest.raises(ValueError, match="changed"): + materializer._verify_output_directory_binding(output, output_fd, identity) + finally: + os.close(output_fd) + + output.mkdir() + output_fd, identity = materializer._open_output_directory(output) + try: + with pytest.raises(ValueError, match="changed"): + materializer._verify_output_directory_binding(output, output_fd, (0, 0)) + finally: + os.close(output_fd) + + +def test_relative_directory_rejects_inode_replacement( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A child directory replaced between stat and open is rejected.""" + root_fd = os.open(tmp_path, materializer._DIRECTORY_OPEN_FLAGS) + identities = iter(((1, 1), (2, 2))) + monkeypatch.setattr( + materializer, + "_directory_identity", + lambda _metadata: next(identities), + ) + try: + with pytest.raises(ValueError, match="changed"): + materializer._open_relative_directory(root_fd, ("child",)) + finally: + os.close(root_fd) + + +def test_output_open_closes_descriptor_when_binding_verification_fails( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A failed binding check does not leak the pinned output descriptor.""" + output = tmp_path / "output" + original_verify = materializer._verify_output_directory_binding + monkeypatch.setattr( + materializer, + "_verify_output_directory_binding", + lambda *_args: (_ for _ in ()).throw(ValueError("changed")), + ) + with pytest.raises(ValueError, match="changed"): + materializer._open_output_directory(output) + monkeypatch.setattr(materializer, "_verify_output_directory_binding", original_verify) + + +def test_new_file_rejects_existing_and_non_regular_targets( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Generated names cannot overwrite existing files or represent directories.""" + parent_fd = os.open(tmp_path, materializer._DIRECTORY_OPEN_FLAGS) + try: + materializer._write_new_file(parent_fd, "created.txt", b"ok") + with pytest.raises(ValueError, match="must not pre-exist"): + materializer._write_new_file(parent_fd, "created.txt", b"again") + + real_fstat = materializer.os.fstat + monkeypatch.setattr( + materializer.os, + "fstat", + lambda file_fd: (tmp_path / "directory-marker").stat(), + ) + (tmp_path / "directory-marker").mkdir() + with pytest.raises(ValueError, match="regular files"): + materializer._write_new_file(parent_fd, "not-regular.txt", b"x") + monkeypatch.setattr(materializer.os, "fstat", real_fstat) + finally: + os.close(parent_fd) + + +def test_new_file_cleans_up_after_write_or_binding_failure( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Partially written or rebound files are removed only when still owned.""" + parent_fd = os.open(tmp_path, materializer._DIRECTORY_OPEN_FLAGS) + try: + monkeypatch.setattr(materializer.os, "write", lambda *_args: 0) + with pytest.raises(OSError, match="no progress"): + materializer._write_new_file(parent_fd, "failed.txt", b"x") + assert not (tmp_path / "failed.txt").exists() + + monkeypatch.undo() + monkeypatch.setattr(materializer.os, "write", lambda *_args: 0) + monkeypatch.setattr( + materializer.os, + "stat", + lambda *_args, **_kwargs: (_ for _ in ()).throw(OSError("stat race")), + ) + with pytest.raises(OSError, match="no progress"): + materializer._write_new_file(parent_fd, "cleanup-error.txt", b"x") + monkeypatch.undo() + + alternate = tmp_path / "alternate.txt" + alternate.write_text("other", encoding="utf-8") + real_stat = materializer.os.stat + calls = 0 + + def rebound_stat(path, *args, **kwargs): + nonlocal calls + if kwargs.get("dir_fd") == parent_fd and path == "rebound.txt" and calls == 0: + calls += 1 + return real_stat(alternate) + return real_stat(path, *args, **kwargs) + + monkeypatch.setattr(materializer.os, "stat", rebound_stat) + with pytest.raises(ValueError, match="changed"): + materializer._write_new_file(parent_fd, "rebound.txt", b"x") + assert not (tmp_path / "rebound.txt").exists() + finally: + os.close(parent_fd) + + def test_main_reports_each_materialized_lock( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, diff --git a/tests/test_npm_nested_metadata_lock_validation.py b/tests/test_npm_nested_metadata_lock_validation.py new file mode 100644 index 000000000..81367069b --- /dev/null +++ b/tests/test_npm_nested_metadata_lock_validation.py @@ -0,0 +1,272 @@ +"""Contracts for npm v2/v3 metadata-only nested package locations.""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from scripts.ci import materialize_base_javascript_packages as materializer + + +_VALID_INTEGRITY = "sha512-" + ("A" * 86) + "==" + + +def _pinned(version: str, package_name: str) -> dict[str, str]: + """Return one exact public-registry package pin.""" + + archive_name = package_name.rsplit("/", 1)[-1] + return { + "version": version, + "resolved": ( + f"https://registry.npmjs.org/{package_name}/-/" + f"{archive_name}-{version}.tgz" + ), + "integrity": _VALID_INTEGRITY, + } + + +def _lock(packages: dict[str, object]) -> bytes: + """Serialize one npm lock fixture as UTF-8 JSON bytes.""" + + return json.dumps( + {"lockfileVersion": 3, "packages": packages}, + sort_keys=True, + ).encode("utf-8") + + +def test_accepts_bandscope_scoped_metadata_through_exact_root_pin() -> None: + """A BandScope-shaped peer location may reuse one exact canonical pin.""" + + packages = { + "": {"name": "bandscope"}, + "node_modules/@types/react-dom": _pinned("19.1.7", "@types/react-dom"), + "apps/desktop/node_modules/@types/react-dom": { + "version": "19.1.7", + "dev": True, + "peer": True, + }, + } + + materializer.validate_head_npm_lock("package-lock.json", _lock(packages)) + + +def test_accepts_unscoped_metadata_and_independently_pinned_nested_version() -> None: + """Metadata reuse and an independently complete nested pin can coexist.""" + + packages = { + "node_modules/react": _pinned("19.1.1", "react"), + "apps/web/node_modules/react": {"version": "19.1.1", "peer": True}, + "node_modules/legacy/node_modules/react": _pinned("18.3.1", "react"), + } + + materializer.validate_head_npm_lock("package-lock.json", _lock(packages)) + + +@pytest.mark.parametrize( + ("packages", "message"), + [ + ( + {"apps/web/node_modules/react": {"version": "19.1.1"}}, + "canonical root pin", + ), + ( + { + "node_modules/react": _pinned("19.1.1", "react"), + "apps/web/node_modules/react": {"version": "19.1.0"}, + }, + "exact canonical version", + ), + ( + { + "node_modules/react": { + "version": "19.1.1", + "resolved": _pinned("19.1.1", "react")["resolved"], + }, + "apps/web/node_modules/react": {"version": "19.1.1"}, + }, + "registry tarball and SHA-512 integrity", + ), + ( + { + "node_modules/react": _pinned("19.1.1", "react"), + "apps/web/node_modules/react": { + "version": "19.1.1", + "resolved": _pinned("19.1.1", "react")["resolved"], + }, + }, + "must not partially declare", + ), + ( + { + "node_modules/react": _pinned("19.1.1", "react"), + "apps/web/node_modules/react": { + "version": "19.1.1", + "integrity": _VALID_INTEGRITY, + }, + }, + "must not partially declare", + ), + ( + { + "node_modules/react": { + **_pinned("19.1.1", "react"), + "resolved": "https://example.invalid/react-19.1.1.tgz", + }, + "apps/web/node_modules/react": {"version": "19.1.1"}, + }, + "must resolve from https://registry.npmjs.org/", + ), + ( + { + "node_modules/react": { + **_pinned("19.1.1", "react"), + "integrity": "sha512-invalid", + }, + "apps/web/node_modules/react": {"version": "19.1.1"}, + }, + "must use one SHA-512 integrity value", + ), + ( + {"apps/web/node_modules/@types": {"version": "1.0.0"}}, + "malformed npm package identity", + ), + ( + {"apps/web/node_modules/@types/react/extra": {"version": "1.0.0"}}, + "malformed npm package identity", + ), + ( + { + "node_modules/react": { + "version": "19.1.1", + "dev": True, + } + }, + "canonical root pin", + ), + ( + { + "node_modules/react": _pinned("19.1.1", "react"), + "apps/web/node_modules/react": {"version": ""}, + }, + "nonempty exact version", + ), + ], +) +def test_rejects_untrusted_metadata_only_nested_locations( + packages: dict[str, object], + message: str, +) -> None: + """Every metadata-only location must close through one exact safe root pin.""" + + with pytest.raises(ValueError, match=message): + materializer.validate_head_npm_lock("package-lock.json", _lock(packages)) + + +def test_regular_base_path_filter_covers_every_rejection_branch( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Tree parsing ignores trees, symlinks, absolute paths, and traversal paths.""" + + def git_stub(_repo_root: Path, *args: str) -> bytes: + assert args[:4] == ("ls-tree", "-r", "-z", "--full-tree") + return b"".join( + ( + b"040000 tree " + (b"0" * 40) + b"\tdirectory\0", + b"120000 blob " + (b"1" * 40) + b"\tsymlink\0", + b"100644 blob " + (b"2" * 40) + b"\t/absolute\0", + b"100644 blob " + (b"3" * 40) + b"\t../escape\0", + b"100644 blob " + (b"4" * 40) + b"\tpackage.json\0", + ) + ) + + monkeypatch.setattr(materializer, "_git", git_stub) + assert materializer._regular_base_paths(tmp_path, "a" * 40) == {"package.json"} + + +@pytest.mark.parametrize( + "lock_document", + [ + {"lockfileVersion": 3}, + { + "lockfileVersion": 3, + "packages": {"packages/missing": {"version": "1.0.0"}}, + }, + ], +) +def test_base_npm_materialization_covers_optional_workspace_metadata( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + lock_document: dict[str, object], +) -> None: + """Missing packages maps and absent workspace manifests stay non-fatal.""" + + monkeypatch.setattr( + materializer, + "_regular_base_paths", + lambda _repo_root, _base_sha: {"package.json", "package-lock.json"}, + ) + + def git_stub(_repo_root: Path, *args: str) -> bytes: + assert args[0] == "show" + target = args[1].split(":", 1)[1] + if target == "package.json": + return b'{"name":"fixture"}\n' + if target == "package-lock.json": + return json.dumps(lock_document).encode("utf-8") + raise AssertionError(target) + + monkeypatch.setattr(materializer, "_git", git_stub) + projects = materializer.base_npm_projects(tmp_path, "a" * 40) + assert len(projects) == 1 + assert set(projects[0][2]) == {"package.json", "package-lock.json"} + + +def test_registry_pin_rejects_non_string_metadata() -> None: + """Registry provenance fields must be exact strings before URL parsing.""" + + with pytest.raises(ValueError, match="must pin a registry tarball"): + materializer._validate_npm_registry_pin( + "package-lock.json", + "node_modules/react", + 123, + _VALID_INTEGRITY, + ) + + +def test_materialize_rejects_symlinked_output_parent( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A parent symlink must never redirect materialized lockfile writes.""" + + trusted_parent = tmp_path / "trusted-parent" + redirected_parent = tmp_path / "redirected-parent" + trusted_parent.mkdir() + redirected_parent.mkdir() + symlink_parent = trusted_parent / "attacker-controlled" + symlink_parent.symlink_to(redirected_parent, target_is_directory=True) + output_dir = symlink_parent / "materialized-locks" + + monkeypatch.setattr(materializer, "base_npm_projects", lambda *_args: []) + monkeypatch.setattr(materializer, "base_pnpm_projects", lambda *_args: []) + + with pytest.raises(ValueError, match="symlink"): + materializer.materialize(tmp_path, "a" * 40, output_dir) + assert not (redirected_parent / "materialized-locks").exists() + + +def test_materialize_rejects_regular_file_output_parent( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A regular-file path component must not be traversed as an output directory.""" + + regular_parent = tmp_path / "regular-parent" + regular_parent.write_text("not a directory\n", encoding="utf-8") + output_dir = regular_parent / "materialized-locks" + + monkeypatch.setattr(materializer, "base_npm_projects", lambda *_args: []) + monkeypatch.setattr(materializer, "base_pnpm_projects", lambda *_args: []) + + with pytest.raises(ValueError, match="path component must be a directory"): + materializer.materialize(tmp_path, "a" * 40, output_dir)