diff --git a/.github/workflows/opencode-review-dispatch.yml b/.github/workflows/opencode-review-dispatch.yml index 0df7a17cc..9c7d781e2 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -1430,6 +1430,18 @@ jobs: writable_npm_cache_dir="$destination" } + trusted_manifest_records_lock_revision() { + local relative_lock="$1" + local head_blob="$2" + jq -e \ + --arg source "$relative_lock" \ + --arg manager "pnpm" \ + --arg revision "${PR_HEAD_SHA,,}" \ + --arg blob "${head_blob,,}" \ + 'any(.[]; .source == $source and (.package_manager | startswith($manager + "@")) and .revision_sha == $revision and .lock_blob == $blob)' \ + /opt/javascript-package-locks/manifest.json >/dev/null 2>&1 + } + trusted_pnpm_lock_matches_base() { local relative_dir local relative_lock @@ -1455,10 +1467,6 @@ jobs: return 1 fi - base_blob="$(trusted_git rev-parse "${PR_BASE_SHA}:${relative_lock}" 2>/dev/null)" || { - echo "::error::Validated base does not contain ${relative_lock}; refusing to trust a PR-added lockfile." - return 1 - } head_blob="$(trusted_git rev-parse "${PR_HEAD_SHA}:${relative_lock}" 2>/dev/null)" || { echo "::error::Validated head does not contain ${relative_lock}." return 1 @@ -1470,10 +1478,27 @@ jobs: echo "::error::Could not hash current pnpm lock ${relative_lock}." return 1 } - if [ "$base_blob" != "$head_blob" ] || [ "$head_blob" != "$worktree_blob" ]; then - echo "::error::Current pnpm lock ${relative_lock} differs from the validated base; refusing --trust-lockfile for PR-controlled dependency resolution." + if [ "$head_blob" != "$worktree_blob" ]; then + echo "::error::Current pnpm lock ${relative_lock} does not match the validated HEAD; refusing --trust-lockfile because the coverage source artifact was tampered with." return 1 fi + + base_blob="$(trusted_git rev-parse "${PR_BASE_SHA}:${relative_lock}" 2>/dev/null)" || base_blob="" + if [ -n "$base_blob" ] && [ "$base_blob" = "$head_blob" ]; then + return 0 + fi + + # A PR-mutated lock is trusted only when the trusted materializer + # recorded this exact lock blob from the validated HEAD revision. + # That record proves the offline store was prefetched from the same + # hash-bounded lock (registry- and integrity-validated at image + # build), so a frozen offline install cannot resolve anything else. + if trusted_manifest_records_lock_revision "$relative_lock" "$head_blob"; then + return 0 + fi + + echo "::error::Current pnpm lock ${relative_lock} differs from the validated base and was not materialized from the validated HEAD; refusing --trust-lockfile for PR-controlled dependency resolution." + return 1 } prepare_writable_pnpm_store() { diff --git a/CHANGELOG.md b/CHANGELOG.md index cef0acda6..04f023e4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ this file. The format follows Keep a Changelog, and versioned releases follow Semantic Versioning where the repository publishes a release. ## [Unreleased] +- Dependency updates now keep coverage evidence when the lock file passes + validation. If validation reports a problem, refresh the lock file and run + the review again before merging. - Route Strix cross-provider fallbacks to explicit direct-OpenAI models (`openai-direct/...`) through the OpenAI inference endpoint instead of inheriting a provider-specific primary base: the workflow now provisions diff --git a/docs/doctoring/opencode-pnpm-head-lock-trust.md b/docs/doctoring/opencode-pnpm-head-lock-trust.md new file mode 100644 index 000000000..e4d918dad --- /dev/null +++ b/docs/doctoring/opencode-pnpm-head-lock-trust.md @@ -0,0 +1,78 @@ +# OpenCode coverage validated head pnpm locks + +검토 기준일: **2026-08-25** + +## Decision + +OpenCode coverage-evidence now trusts a PR-mutated `pnpm-lock.yaml` only when +the trusted materializer recorded that exact lock blob from the validated HEAD +revision. The sandbox keeps three integrity boundaries and adds a fourth: + +1. The coverage source artifact must hash-match the validated `PR_HEAD_SHA` + lock (tamper evidence between artifact download and use). +2. An unchanged base lock (base blob equals head blob) remains trusted exactly + as before. +3. A head-mutated lock is trusted only when + `/opt/javascript-package-locks/manifest.json` records + `source`, `revision_sha == PR_HEAD_SHA`, and `lock_blob` for this project — + proving the offline store was prefetched from the same hash-bounded lock at + image build time. +4. Before materialization, `validate_head_pnpm_lock` fails closed unless every + package entry pins one SHA-512 SRI, every tarball URL is an HTTPS + `registry.npmjs.org` URL without userinfo, port, query, or fragment, and any + workspace link target is a relative in-project directory. VCS or file + sources are refused. + +The npm path already followed this pattern through +`validate_head_npm_lock`; the pnpm path now mirrors it. `--offline`, +`--frozen-lockfile`, and lifecycle-hook suppression remain mandatory, so a +mutated lock can never fetch anything outside the store that was verified +against the registry's own integrity metadata during image build (npm, n.d.; +pnpm, n.d.). + +## Root-cause analysis + +1. The previous gate required base blob == head blob == worktree blob for + every pnpm project. Any dependency-raising pull request necessarily mutates + the lockfile, so such PRs failed coverage-evidence with "Current pnpm lock + differs from the validated base" regardless of content quality. +2. The failure was not hypothetical: ContextualWisdomLab/inkspan#373 (a + transitive security-floor raise for fast-uri, nanoid, and postcss) carried + fully green repository-owned checks but could never satisfy this gate, + leaving the security fix unmergeable while Dependabot alerts stayed open. +3. The image build already consumed strictly registry/hash-bounded inputs from + the live-validated HEAD (`materialize_base_javascript_packages.py --head-sha`), + so refusing head-mutated pnpm locks added no integrity guarantee that the + build did not already enforce; it only blocked legitimate dependency work. + +## Remediation + +- Materializers validate changed head pnpm locks with the same fail-closed + posture as npm locks before anything enters the networked build context. +- The sandbox consults the trusted manifest record instead of refusing every + mutation, keeping tamper evidence against `PR_HEAD_SHA`. +- Repositories regain the ability to ship audited dependency updates through + reviewed pull requests instead of forcing direct-to-main writes. + +Independent OpenCode, Strix, and Noema review remain authorization gates. This +change does not approve, merge, or weaken hash-pinned Python installs, registry +allowlists, or the networkless PR sandbox. + +## APA 7th references + +MITRE. (2026). *CWE-494: Download of code without integrity check*. +https://cwe.mitre.org/data/definitions/494.html + +National Institute of Standards and Technology. (2022). *Secure software +development framework (SSDF) version 1.1: Recommendations for mitigating the +risk of software vulnerabilities* (NIST Special Publication 800-218). +https://doi.org/10.6028/NIST.SP.800-218 + +npm, Inc. (n.d.). *Package lock specification: integrity fields*. npm Docs. +Retrieved August 25, 2026, from https://docs.npmjs.com/cli/v10/configuring-npm/package-lock-json + +Open Worldwide Application Security Project. (2025). *OWASP Top 10: A06 +— vulnerable and outdated components*. https://owasp.org/Top10/A06_2021-Vulnerable_and_Outdated_Components/ + +pnpm. (n.d.). *Settings: lockfile and frozen-lockfile*. pnpm Docs. +Retrieved August 25, 2026, from https://pnpm.io/settings diff --git a/scripts/ci/materialize_base_javascript_packages.py b/scripts/ci/materialize_base_javascript_packages.py index 407c17aa1..3593fd6a0 100644 --- a/scripts/ci/materialize_base_javascript_packages.py +++ b/scripts/ci/materialize_base_javascript_packages.py @@ -24,8 +24,33 @@ PNPM_SPEC_RE = re.compile(r"^pnpm@[0-9]+\.[0-9]+\.[0-9]+(?:[+-][A-Za-z0-9._+-]+)?$") PNPM_BASE_INPUT_NAMES = ("package.json", "pnpm-workspace.yaml", ".pnpmfile.cjs") NPM_LOCK_NAMES = ("npm-shrinkwrap.json", "package-lock.json") +PNPM_LOCK_NAME = "pnpm-lock.yaml" NPM_REGISTRY_HOST = "registry.npmjs.org" SHA512_SRI_RE = re.compile(r"^sha512-[A-Za-z0-9+/]{86}==$") +PNPM_PACKAGE_ENTRY_RE = re.compile(r"^ ([^ #][^:]*):(?:\s.*)?$") +PNPM_RESOLUTION_RE = re.compile(r"resolution:\s*\{(.*)\}\s*$") +PNPM_TARBALL_RE = re.compile(r"tarball:\s*([^,\s}]+)") +PNPM_INTEGRITY_RE = re.compile(r"integrity:\s*([^,\s}]+)") +PNPM_DIRECTORY_RE = re.compile(r"directory:\s*\"?([^,\"}]+)\"?") +PNPM_LINK_TRUE_RE = re.compile(r"link:\s*true\b") + + +def _github_actions_escape(value: object) -> str: + """Escape untrusted text before writing it to a GitHub Actions log. + + GitHub Actions recognizes workflow commands in log lines. Repository paths + and git diagnostics can contain command delimiters, newlines, or percent + escapes when a pull request controls the tree, so diagnostics must never be + emitted verbatim. The manifest itself remains raw; this helper only protects + the human-readable CLI output. + """ + return ( + str(value) + .replace("%", "%25") + .replace("\r", "%0D") + .replace("\n", "%0A") + .replace(":", "%3A") + ) def _git(repo_root: pathlib.Path, *args: str) -> bytes: @@ -340,6 +365,147 @@ def validate_head_npm_lock(lock_path: str, lock_content: bytes) -> None: ) +def _validate_pnpm_tarball_url( + lock_path: str, package_key: str, tarball_url: str +) -> None: + """Fail closed unless one pnpm tarball URL is an npm-registry HTTPS URL.""" + parsed = urllib.parse.urlsplit(tarball_url) + try: + parsed_port = parsed.port + except ValueError as exc: + raise ValueError( + f"current-head pnpm lock {lock_path} package {package_key} has an invalid tarball 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 pnpm lock {lock_path} package {package_key} must resolve from https://{NPM_REGISTRY_HOST}/" + ) + + +def validate_head_pnpm_lock(lock_path: str, lock_content: bytes) -> None: + """Fail closed unless a changed HEAD pnpm lock is registry- and hash-bounded. + + The validator is intentionally line-based and standard-library-only: pnpm + lockfiles always emit each package's ``resolution`` as a single-line inline + mapping, so scanning those lines covers every fetched artifact while never + introducing a YAML parser dependency into the trusted materializer. + """ + try: + text = lock_content.decode("utf-8") + except UnicodeDecodeError as exc: + raise ValueError( + f"current-head pnpm lock {lock_path} is invalid UTF-8: {exc}" + ) from exc + if not text.strip(): + raise ValueError(f"current-head pnpm lock {lock_path} is empty") + + in_packages_section = False + package_entry_count = 0 + current_package_key = "" + current_resolution_seen = False + + for raw_line in text.splitlines(): + line = raw_line.rstrip() + if not line or line.lstrip().startswith("#"): + continue + indent = len(line) - len(line.lstrip(" ")) + stripped = line.strip() + + if indent == 0 and stripped.endswith(":"): + in_packages_section = stripped == "packages:" + continue + if not in_packages_section: + continue + + if indent == 2: + entry_match = PNPM_PACKAGE_ENTRY_RE.match(line) + if entry_match is None: + raise ValueError( + f"current-head pnpm lock {lock_path} contains an unexpected " + f"two-space entry {stripped!r}" + ) + if current_package_key and not current_resolution_seen: + raise ValueError( + f"current-head pnpm lock {lock_path} package {current_package_key} " + "has no resolution entry" + ) + current_package_key = entry_match.group(1).strip() + package_entry_count += 1 + current_resolution_seen = False + continue + + if current_package_key and stripped.startswith("resolution:"): + resolution_match = PNPM_RESOLUTION_RE.search(stripped) + if resolution_match is None: + raise ValueError( + f"current-head pnpm lock {lock_path} package {current_package_key} " + "has a multi-line or malformed resolution mapping" + ) + resolution_body = resolution_match.group(1) + integrity_match = PNPM_INTEGRITY_RE.search(resolution_body) + link_match = PNPM_LINK_TRUE_RE.search(resolution_body) + directory_match = PNPM_DIRECTORY_RE.search(resolution_body) + if link_match is not None: + if directory_match is None: + raise ValueError( + f"current-head pnpm lock {lock_path} workspace link " + f"{current_package_key} must carry a relative directory target" + ) + directory_value = directory_match.group(1).strip().strip('"') + directory_candidate = pathlib.PurePosixPath(directory_value) + if ( + directory_candidate.is_absolute() + or ".." in directory_candidate.parts + or "node_modules" in directory_candidate.parts + ): + raise ValueError( + f"current-head pnpm lock {lock_path} workspace link " + f"{current_package_key} has an unsafe directory target" + ) + elif integrity_match is None or not SHA512_SRI_RE.fullmatch( + integrity_match.group(1) + ): + raise ValueError( + f"current-head pnpm lock {lock_path} package {current_package_key} " + "must pin exactly one SHA-512 integrity value" + ) + tarball_match = PNPM_TARBALL_RE.search(resolution_body) + if tarball_match is not None: + _validate_pnpm_tarball_url( + lock_path, current_package_key, tarball_match.group(1) + ) + current_resolution_seen = True + continue + + if current_package_key and ( + stripped.startswith("tarball:") or stripped.startswith("git+") + ): + raise ValueError( + f"current-head pnpm lock {lock_path} package {current_package_key} " + "carries an out-of-band fetch source" + ) + + if current_package_key and not current_resolution_seen: + raise ValueError( + f"current-head pnpm lock {lock_path} package {current_package_key} " + "has no resolution entry" + ) + if package_entry_count == 0: + raise ValueError( + f"current-head pnpm lock {lock_path} contains no package entries" + ) + + def materialize( repo_root: pathlib.Path, base_sha: str, @@ -356,6 +522,7 @@ def materialize( 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] = {} + base_pnpm_blobs: dict[str, str] = {} for source_path, package_manager, base_inputs in ( base_pnpm_projects(repo_root, base_sha) + base_npm ): @@ -371,6 +538,8 @@ def materialize( ) if source_path in base_npm_paths: base_npm_blobs[source_path] = lock_blob + else: + base_pnpm_blobs[source_path] = lock_blob if head_sha is not None: if not SHA_RE.fullmatch(head_sha): @@ -392,6 +561,22 @@ def materialize( head_blob, ) ) + for source_path, package_manager, head_inputs in base_pnpm_projects( + repo_root, head_sha + ): + head_blob = _lock_blob_sha(repo_root, head_sha, source_path) + if base_pnpm_blobs.get(source_path) == head_blob: + continue + validate_head_pnpm_lock(source_path, head_inputs[PNPM_LOCK_NAME]) + projects.append( + ( + source_path, + package_manager, + head_inputs, + head_sha.lower(), + head_blob, + ) + ) for index, ( source_path, @@ -442,7 +627,8 @@ def main(argv: list[str] | None = None) -> int: ) except (OSError, RuntimeError, ValueError) as exc: print( - f"::error::Could not materialize base JavaScript package locks: {exc}", + "::error::Could not materialize base JavaScript package locks: " + f"{_github_actions_escape(exc)}", file=sys.stderr, ) return 1 @@ -451,9 +637,11 @@ def main(argv: list[str] | None = None) -> int: for entry in manifest: print( "Materialized trusted JavaScript lock " - f"{entry['source']} for {entry['package_manager']} " - f"from {entry['revision_sha']} as " - f"{entry['directory']}/{pathlib.PurePosixPath(entry['source']).name}." + f"{_github_actions_escape(entry['source'])} for " + f"{_github_actions_escape(entry['package_manager'])} from " + f"{_github_actions_escape(entry['revision_sha'])} as " + f"{_github_actions_escape(entry['directory'])}/" + f"{_github_actions_escape(pathlib.PurePosixPath(entry['source']).name)}." ) else: print( diff --git a/tests/test_materialize_base_javascript_packages.py b/tests/test_materialize_base_javascript_packages.py index 62b954259..ad3a88e08 100644 --- a/tests/test_materialize_base_javascript_packages.py +++ b/tests/test_materialize_base_javascript_packages.py @@ -191,6 +191,63 @@ def test_materializes_only_exact_base_pnpm_inputs(tmp_path: Path) -> None: ) +def test_workflow_accepts_versioned_pnpm_materializer_manifest_record( + tmp_path: Path, +) -> None: + """The workflow predicate must accept the exact pnpm spec materialize emits.""" + repo, revision_sha = fixture_repo(tmp_path) + manifest = materializer.materialize(repo, revision_sha, tmp_path / "output") + record = manifest[0] + assert record["package_manager"] == "pnpm@11.5.3" + + workflow = Path(".github/workflows/opencode-review-dispatch.yml").read_text( + encoding="utf-8" + ) + start = workflow.index("trusted_manifest_records_lock_revision() {") + end = workflow.index("\n }", start) + predicate_line = next( + line.strip() + for line in workflow[start:end].splitlines() + if line.strip().startswith("'any(.[];") + ) + predicate = predicate_line.split("' \\", 1)[0][1:] + command = [ + "jq", + "-e", + "--arg", + "source", + record["source"], + "--arg", + "manager", + "pnpm", + "--arg", + "revision", + record["revision_sha"], + "--arg", + "blob", + record["lock_blob"], + predicate, + ] + accepted = subprocess.run( + command, + input=json.dumps(manifest), + capture_output=True, + text=True, + check=False, + ) + assert accepted.returncode == 0, accepted.stderr + + wrong_manager = [{**record, "package_manager": "npm"}] + rejected = subprocess.run( + command, + input=json.dumps(wrong_manager), + capture_output=True, + text=True, + check=False, + ) + assert rejected.returncode == 1, rejected.stderr + + def test_materializes_only_exact_base_npm_inputs(tmp_path: Path) -> None: """PR-modified npm metadata cannot enter the networked build context.""" repo, base_sha = npm_fixture_repo(tmp_path) @@ -736,6 +793,49 @@ def test_main_reports_materialized_lock( ) +def test_main_escapes_pull_request_paths_in_github_logs( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], +) -> None: + """A malicious repository path cannot emit a second Actions command.""" + malicious_source = "::set-output name=leak::value\nfrontend/package-lock.json" + monkeypatch.setattr( + materializer, + "materialize", + lambda *_args, **_kwargs: [ + { + "directory": "project-000", + "lock_blob": "b" * 40, + "package_manager": "npm", + "revision_sha": "a" * 40, + "source": malicious_source, + } + ], + ) + + assert ( + materializer.main( + [ + "--repo-root", + str(tmp_path), + "--base-sha", + "a" * 40, + "--output-dir", + str(tmp_path / "output"), + ] + ) + == 0 + ) + + output = capsys.readouterr().out + assert "\n::set-output" not in output + assert ( + "%3A%3Aset-output name=leak%3A%3Avalue%0Afrontend/package-lock.json" + in output + ) + + def test_main_reports_empty_base( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, @@ -818,3 +918,300 @@ def test_script_entrypoint_exits_through_main( with pytest.raises(SystemExit) as raised: runpy.run_path(str(module_path), run_name="__main__") assert raised.value.code == 1 + + +def _bounded_head_pnpm_lock() -> str: + """Return a realistic registry- and integrity-bounded pnpm lockfile body.""" + return ( + "lockfileVersion: '9.0'\n" + "\n" + "settings:\n" + " autoInstallPeers: true\n" + "\n" + "packages:\n" + " fast-uri@3.1.5:\n" + " resolution: {integrity: sha512-" + ("B" * 86) + "==}\n" + "\n" + "snapshots:\n" + " fast-uri@3.1.5: {}\n" + ) + + +def pnpm_head_fixture_repo(tmp_path: Path) -> tuple[Path, str]: + """Create a pnpm repository whose head raises a dependency floor.""" + repo = tmp_path / "pnpm-head-repo" + repo.mkdir() + git(repo, "init") + git(repo, "config", "user.name", "Test") + git(repo, "config", "user.email", "test@example.invalid") + + frontend = repo / "frontend" + frontend.mkdir() + (frontend / "package.json").write_text( + json.dumps({"packageManager": "pnpm@11.5.3"}) + "\n", + encoding="utf-8", + ) + (frontend / "pnpm-lock.yaml").write_text( + "lockfileVersion: '9.0'\n" + "packages:\n" + " fast-uri@3.1.4:\n" + " resolution: {integrity: sha512-" + ("A" * 86) + "==}\n" + "\n" + "snapshots:\n" + " fast-uri@3.1.4: {}\n", + encoding="utf-8", + ) + git(repo, "add", ".") + git(repo, "commit", "-m", "pnpm base") + base_sha = git(repo, "rev-parse", "HEAD") + + (repo / "frontend" / "package.json").write_text( + json.dumps({"packageManager": "pnpm@11.5.3"}) + "\n", + encoding="utf-8", + ) + (repo / "frontend" / "pnpm-lock.yaml").write_text( + _bounded_head_pnpm_lock(), + encoding="utf-8", + ) + git(repo, "add", ".") + git(repo, "commit", "-m", "pnpm bounded head") + return repo, base_sha + + +def test_materializes_strict_changed_head_pnpm_lock_after_base( + tmp_path: Path, +) -> None: + """A registry- and hash-bounded head pnpm lock joins the trusted store.""" + repo, base_sha = pnpm_head_fixture_repo(tmp_path) + head_sha = git(repo, "rev-parse", "HEAD") + output = tmp_path / "output" + + manifest = materializer.materialize(repo, base_sha, output, head_sha=head_sha) + + assert {entry["revision_sha"] for entry in manifest} == {base_sha, head_sha} + head_entry = next( + entry + for entry in manifest + if entry["revision_sha"] == head_sha and entry["source"] == "frontend/pnpm-lock.yaml" + ) + assert head_entry["lock_blob"] == git( + repo, "rev-parse", f"{head_sha}:frontend/pnpm-lock.yaml" + ) + assert ( + output / head_entry["directory"] / "pnpm-lock.yaml" + ).read_text(encoding="utf-8") == _bounded_head_pnpm_lock() + + +def test_unchanged_head_pnpm_lock_is_not_materialized_twice( + tmp_path: Path, +) -> None: + """An unchanged head pnpm lock reuses the exact base cache entry.""" + repo, base_sha = pnpm_head_fixture_repo(tmp_path) + + manifest = materializer.materialize( + repo, + base_sha, + tmp_path / "output", + head_sha=base_sha, + ) + + assert len(manifest) == 1 + assert manifest[0]["revision_sha"] == base_sha + + +def test_rejects_malformed_changed_head_pnpm_lock(tmp_path: Path) -> None: + """A head pnpm lock without an integrity pin cannot enter the store.""" + repo, base_sha = pnpm_head_fixture_repo(tmp_path) + (repo / "frontend" / "pnpm-lock.yaml").write_text( + "lockfileVersion: '9.0'\npackages:\n hostile@9.9.9: {}\n", + encoding="utf-8", + ) + git(repo, "add", ".") + git(repo, "commit", "-m", "hostile pnpm head") + head_sha = git(repo, "rev-parse", "HEAD") + + with pytest.raises(ValueError, match="has no resolution entry"): + materializer.materialize(repo, base_sha, tmp_path / "output", head_sha=head_sha) + + +def test_rejects_off_registry_tarball_in_changed_head_pnpm_lock( + tmp_path: Path, +) -> None: + """A non-npmjs tarball source is refused before image build.""" + repo, base_sha = pnpm_head_fixture_repo(tmp_path) + (repo / "frontend" / "pnpm-lock.yaml").write_text( + "lockfileVersion: '9.0'\n" + "packages:\n" + " evil@1.0.0:\n" + " resolution: {tarball: https://evil.invalid/evil/-/evil-1.0.0.tgz," + " integrity: sha512-" + ("C" * 86) + "==}\n", + encoding="utf-8", + ) + git(repo, "add", ".") + git(repo, "commit", "-m", "off-registry tarball head") + head_sha = git(repo, "rev-parse", "HEAD") + + with pytest.raises(ValueError, match="must resolve from https"): + materializer.materialize(repo, base_sha, tmp_path / "output", head_sha=head_sha) + + +def test_rejects_unsafe_workspace_link_in_changed_head_pnpm_lock( + tmp_path: Path, +) -> None: + """Workspace links must stay inside the project tree.""" + repo, base_sha = pnpm_head_fixture_repo(tmp_path) + (repo / "frontend" / "pnpm-lock.yaml").write_text( + "lockfileVersion: '9.0'\n" + "packages:\n" + " escape@1.0.0:\n" + " resolution: {directory: ../../secrets, link: true}\n", + encoding="utf-8", + ) + git(repo, "add", ".") + git(repo, "commit", "-m", "escaping workspace link head") + head_sha = git(repo, "rev-parse", "HEAD") + + with pytest.raises(ValueError, match="unsafe directory target"): + materializer.materialize(repo, base_sha, tmp_path / "output", head_sha=head_sha) + + +def test_validate_head_pnpm_lock_accepts_bounded_lock() -> None: + """The validator accepts exactly the lock shape the store can prefetch.""" + materializer.validate_head_pnpm_lock( + "pnpm-lock.yaml", _bounded_head_pnpm_lock().encode("utf-8") + ) + + +def test_validate_head_pnpm_lock_accepts_multi_key_resolution_mappings() -> None: + """Comma-delimited inline values must retain exact token boundaries.""" + integrity = "sha512-" + ("E" * 86) + "==" + tarball = "https://registry.npmjs.org/example/-/example-1.0.0.tgz" + for resolution in ( + f"tarball: {tarball}, integrity: {integrity}", + f"integrity: {integrity}, tarball: {tarball}", + ): + content = ( + "lockfileVersion: '9.0'\n" + "packages:\n" + " example@1.0.0:\n" + f" resolution: {{{resolution}}}\n" + ) + materializer.validate_head_pnpm_lock( + "pnpm-lock.yaml", content.encode("utf-8") + ) + + +def test_validate_head_pnpm_lock_accepts_fetch_words_in_deprecation_text() -> None: + """Metadata prose cannot be reclassified as an artifact source declaration.""" + integrity = "sha512-" + ("F" * 86) + "==" + content = ( + "lockfileVersion: '9.0'\n" + "packages:\n" + " example@1.0.0:\n" + f" resolution: {{integrity: {integrity}}}\n" + " deprecated: migrate from git+https://example.invalid/source; " + "the tarball: note is informational only\n" + ) + materializer.validate_head_pnpm_lock( + "pnpm-lock.yaml", content.encode("utf-8") + ) + + +@pytest.mark.parametrize( + ("directory", "accepted"), + ( + ("packages/example", True), + ("/tmp/example", False), + ("../example", False), + ("node_modules/example", False), + ), +) +def test_validate_head_pnpm_lock_bounds_workspace_directory_variants( + directory: str, accepted: bool +) -> None: + """Workspace targets are admitted only when they remain project-relative.""" + content = ( + "lockfileVersion: '9.0'\n" + "packages:\n" + " workspace@example:\n" + f" resolution: {{directory: {directory}, link: true}}\n" + ).encode("utf-8") + + if accepted: + materializer.validate_head_pnpm_lock("pnpm-lock.yaml", content) + else: + with pytest.raises(ValueError, match="unsafe directory target"): + materializer.validate_head_pnpm_lock("pnpm-lock.yaml", content) + + +@pytest.mark.parametrize( + ("content", "message"), + ( + (b"\xff", "invalid UTF-8"), + (b"lockfileVersion: '9.0'\npackages:\n : malformed\n", "unexpected two-space entry"), + ( + b"lockfileVersion: '9.0'\npackages:\n first@1.0.0:\n second@1.0.0:\n", + "first@1.0.0 has no resolution entry", + ), + ( + b"lockfileVersion: '9.0'\npackages:\n malformed@1.0.0:\n resolution:\n", + "multi-line or malformed resolution mapping", + ), + ( + b"lockfileVersion: '9.0'\npackages:\n workspace@example:\n resolution: {link: true}\n", + "must carry a relative directory target", + ), + ( + b"lockfileVersion: '9.0'\npackages:\n weak@1.0.0:\n resolution: {integrity: sha256-weak}\n", + "must pin exactly one SHA-512 integrity value", + ), + ( + b"lockfileVersion: '9.0'\npackages:\n tarball@1.0.0:\n tarball: https://registry.npmjs.org/tarball/-/tarball-1.0.0.tgz\n", + "carries an out-of-band fetch source", + ), + ( + b"lockfileVersion: '9.0'\npackages:\n git@1.0.0:\n git+https://example.invalid/git.git\n", + "carries an out-of-band fetch source", + ), + (b"lockfileVersion: '9.0'\npackages:\n", "contains no package entries"), + ), +) +def test_validate_head_pnpm_lock_rejects_malformed_structures( + content: bytes, message: str +) -> None: + """Every structural fail-closed path remains executable evidence.""" + with pytest.raises(ValueError, match=message): + materializer.validate_head_pnpm_lock("pnpm-lock.yaml", content) + + +def test_validate_head_pnpm_lock_rejects_invalid_tarball_port() -> None: + """A non-numeric registry port cannot escape URL validation as metadata.""" + integrity = "sha512-" + ("G" * 86) + "==" + content = ( + "lockfileVersion: '9.0'\n" + "packages:\n" + " invalid-port@1.0.0:\n" + " resolution: {tarball: " + "https://registry.npmjs.org:not-a-port/invalid-port/-/invalid-port-1.0.0.tgz, " + f"integrity: {integrity}}}\n" + ) + with pytest.raises(ValueError, match="invalid tarball URL"): + materializer.validate_head_pnpm_lock( + "pnpm-lock.yaml", content.encode("utf-8") + ) + + +def test_validate_head_pnpm_lock_rejects_empty_and_git_sources() -> None: + """Empty locks and VCS fetch sources fail closed.""" + with pytest.raises(ValueError, match="empty"): + materializer.validate_head_pnpm_lock("pnpm-lock.yaml", b"") + with pytest.raises(ValueError, match="must resolve from https"): + content = ( + "lockfileVersion: '9.0'\n" + "packages:\n" + " gitdep@1.0.0:\n" + " resolution: {tarball: https://codeload.github.com/example/example/tar.gz/abc123," + " integrity: sha512-" + ("D" * 86) + "==}\n" + " # git+https://example.invalid/example.git\n" + ) + materializer.validate_head_pnpm_lock("pnpm-lock.yaml", content.encode("utf-8")) diff --git a/tests/test_opencode_agent_contract.py b/tests/test_opencode_agent_contract.py index f0b1af470..1e3756169 100644 --- a/tests/test_opencode_agent_contract.py +++ b/tests/test_opencode_agent_contract.py @@ -636,6 +636,30 @@ def test_opencode_target_coverage_materializes_only_after_authorized_dispatch(): ) assert 'hash-object --no-filters -- "$relative_lock"' not in measure_step assert "refusing --trust-lockfile for PR-controlled dependency resolution" in measure_step + # A PR-mutated pnpm lock is trusted only when the trusted materializer + # recorded the exact head blob from the validated HEAD revision; the + # sandbox must consult that manifest record instead of refusing every + # dependency-changing pull request. + pnpm_trust_block = measure_step.split( + "trusted_pnpm_lock_matches_base() {", 1 + )[1].split("prepare_writable_pnpm_store()", 1)[0] + assert ( + "differs from the validated base and was not materialized from " + "the validated HEAD" + ) in pnpm_trust_block + assert ( + "does not match the validated HEAD; refusing --trust-lockfile " + "because the coverage source artifact was tampered with" + ) in pnpm_trust_block + assert "trusted_manifest_records_lock_revision" in pnpm_trust_block + assert '--arg manager "pnpm"' in measure_step + assert '.package_manager | startswith($manager + "@")' in measure_step + assert "/opt/javascript-package-locks/manifest.json" in measure_step + assert ".revision_sha == $revision and .lock_blob == $blob" in measure_step + # Manifest records normalize Git object identities to lowercase; pnpm trust + # must match the npm path even when an input SHA uses uppercase hex. + assert '--arg revision "${PR_HEAD_SHA,,}"' in measure_step + assert '--arg blob "${head_blob,,}"' in measure_step assert "prepare_writable_pnpm_store()" in measure_step assert ( 'destination="$(mktemp -d /tmp/opencode-pnpm-store.XXXXXX)"' diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index a5d25379a..7e91ab472 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -19,7 +19,7 @@ DOCTORING_RECORD = Path("docs/doctoring/hourly-nvidia-nim-autofix.md") CHANGELOG = Path("CHANGELOG.md") REVIEW_DISPATCH_WORKFLOW = Path(".github/workflows/opencode-review-dispatch.yml") -REVIEW_DISPATCH_BLOB_SHA = "0df7a17cc72a79585cec169c8299e0646f93ab02" +REVIEW_DISPATCH_BLOB_SHA = "9c7d781e2dde3255963d7b9bd606c5ba80ac3621" def _workflow_text(path: Path) -> str: