Skip to content

fix(coverage): validate nested npm metadata through canonical pins - #807

Draft
seonghobae wants to merge 52 commits into
mainfrom
fix/npm-nested-metadata-lock-validation
Draft

fix(coverage): validate nested npm metadata through canonical pins#807
seonghobae wants to merge 52 commits into
mainfrom
fix/npm-nested-metadata-lock-validation

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Problem

Central changed-head npm-lock validation historically required every non-link node_modules location to repeat its own registry tarball and SHA-512 integrity. npm v2/v3 can serialize workspace or peer locations with version/classification metadata while one canonical location carries the artifact pin, so valid BandScope-style entries were blocked despite an exact canonical root pin.

Exact current stack

  • exact head: 7b1a08f947e960e40be46f91227676066188e4d1;
  • protected base/main: e71fdab2ab088001f218765ecb5e3b7fabfee11a;
  • main is an ancestor and GitHub reports the five-file slice mergeable;
  • all temporary writer workflows and staged patch artifacts are absent;
  • predecessor-head checks and reviews are stale.

Permanent implementation

  • Derive scoped or unscoped package identity only from segments after the final node_modules.
  • Accept metadata-only nested locations only when node_modules/<same exact identity> in the same authenticated lock is a non-link canonical entry with identical exact version, a public registry.npmjs.org HTTPS tarball, and one canonical SHA-512 SRI value.
  • Validate independently complete nested pins without forcing them to reuse the canonical version.
  • Reject malformed identities, canonical metadata-only entries, missing canonical metadata, version mismatch, partial/conflicting pins, links, userinfo, every explicit port including :443, query/fragment data, hostile origins, non-tarball URLs, and invalid SRI.
  • Consume the authenticated lock unchanged and never fetch, repair, infer, or rewrite dependency metadata.
  • Copy workspace manifests only when they are regular blobs in the authenticated base tree.

Exact-head acceptance evidence

For 7b1a08f947e960e40be46f91227676066188e4d1, all exact-head workflows completed successfully:

  • npm Nested Metadata Validation Quality CI 31152353756;
  • CodeQL PR 31152353761;
  • Python Security 31152353801;
  • SAST Semgrep 31152353800;
  • Security Scan 31152353834;
  • Secret Scan 31152353762;
  • OSV-Scanner PR 31152354083;
  • Scorecard PR 31152353799;
  • SBOM Generation 31152353804;
  • CodeRabbit commit status: success.

Focused and complete central suites prove 100% production statement and branch coverage, complete production docstrings, compilation, authenticated-base workspace handling, malformed URL/port behavior, and no temporary writer scope. All inline findings are resolved or outdated; zero actionable unresolved thread remains.

Formal exact-current-head CodeRabbit/OpenCode/Noema review and a qualifying independent non-author approval remain required. Queued, cancelled, predecessor-head, status-only, author-only, local-only, or temporary-workflow evidence is not accepted.

Standards and merge gate

docs/doctoring/npm-nested-metadata-canonical-pins.md records the trust boundary, explicit-port policy, failure and rollback behavior, and APA 7 references to npm package-lock format and Subresource Integrity. Merge only after exact-current-head automated review, qualifying independent non-author approval, zero unresolved actionable findings, and branch protection succeed without bypass.

Refs ContextualWisdomLab/bandscope#751 and #748.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5cb87dda-668b-40af-87d2-c10064bb1900

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

npm lockfile v2/v3의 중첩 metadata-only 위치에 대한 canonical pin 검증을 추가했습니다. 관련 회귀 테스트, 정책 문서, 변경 이력과 품질 CI도 추가했습니다.

Changes

npm 중첩 메타데이터 검증

Layer / File(s) Summary
Lockfile 핀 검증 구현
scripts/ci/materialize_base_javascript_packages.py
패키지 identity를 추출하고 HTTPS npm registry tarball과 단일 SHA-512 integrity를 검증합니다. 중첩 metadata-only 위치는 canonical root pin과 버전이 같을 때 허용합니다.
검증 회귀 테스트
tests/test_npm_nested_metadata_lock_validation.py
정상적인 root pin 재사용과 독립 pin을 검증합니다. 잘못된 identity, 버전, URL, integrity, 경로와 선택적 workspace metadata 처리도 검증합니다.
정책 문서와 품질 CI
docs/doctoring/npm-nested-metadata-canonical-pins.md, CHANGELOG.md, .github/workflows/npm-nested-metadata-validation-quality-ci.yml
canonical pin 정책과 거부 조건을 문서화합니다. Python 호환성, 컴파일, docstring, 커버리지와 git diff --check 검사를 CI에 추가합니다.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GitHubActions as GitHub Actions
  participant Tests as npm lock validation tests
  participant Materializer as materialize_base_javascript_packages.py
  participant Lockfile as npm lockfile
  GitHubActions->>Tests: 대상 테스트와 커버리지 실행
  Tests->>Materializer: lockfile materialization 호출
  Materializer->>Lockfile: package metadata 읽기
  Materializer-->>Tests: pin 검증 결과 반환
  Tests-->>GitHubActions: 테스트 및 커버리지 결과 보고
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 canonical pin을 통한 중첩 npm 메타데이터 검증 변경을 정확하고 간결하게 설명합니다.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/npm-nested-metadata-lock-validation

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Contributor Author

@opencode-agent

Implement the bounded GREEN slice on exact current head cf014e8441e8ffbe8b41d93585ceb5c1a3d5bd4a, then remove .github/workflows/materialize-npm-nested-metadata-validation.yml from the final tree. Do not add any other one-shot or branch-writing workflow.

Apply the permanent tests and minimal production changes already specified by the branch contract:

  • accept a metadata-only nested npm v2/v3 node_modules location only when one canonical root package has the same exact package identity/version and one valid public npm-registry .tgz URL plus SHA-512 SRI;
  • reject malformed scoped identities, missing/empty version, partial resolved/integrity, version mismatch, missing canonical metadata, non-registry URL, invalid SRI, and conflicting complete pins;
  • preserve path/link, traversal, userinfo, port, query, fragment, lock-version, exact-head, and immutable-materialization boundaries;
  • consume the validated lock unchanged;
  • add BandScope-shaped positive/negative regressions, APA 7 doctoring, and CHANGELOG evidence;
  • run focused RED/GREEN tests, complete central tests, 100% production statement/branch coverage, 100% public docstrings, Ruff, compilation, and git diff --check.

Keep Draft and do not approve, merge, release, or change reviewer credentials.

Copy link
Copy Markdown
Contributor Author

Release blocker: current branch contains only a trigger and branch-materialization workflows (.github/npm-nested-metadata.trigger, materialize-npm-nested-metadata-validation.yml, trigger-npm-nested-metadata-materializer.yml). Do not run or merge a PR-controlled workflow that rewrites and pushes its own reviewed source.

@opencode-agent address

Remove every trigger, self-removing materializer, encoded patch, and branch-writing helper from the final tree. Preserve the test-first intent, but publish the four permanent implementation/test/doctoring/changelog files through ordinary reviewed commits. The final production rule may accept a metadata-only nested npm v2/v3 location only when one canonical root package with the same normalized identity and exact version has a valid public npm-registry tarball and SHA-512 SRI. Continue to reject missing canonical metadata, version mismatch, partial pins, malformed scoped identities, non-registry origins, userinfo/query/fragment/port abuse, invalid SRI, conflicting complete pins, unsafe paths, and links. Consume the lock unchanged; do not repair or synthesize metadata. Require exact-head Python 3.10/3.14 tests, 100% production statement/branch/docstring evidence, compilation, all security/supply-chain checks, current-head review, and independent approval before Ready.

Copy link
Copy Markdown
Contributor Author

@jules Replace the trigger/materializer-only scope on exact live head 4b2fceea5cbbff929623fec85b371f7f0bbe2898 with ordinary reviewed permanent implementation, tests, APA 7 doctoring, and CHANGELOG commits. Keep Draft.

Delete .github/npm-nested-metadata.trigger, .github/workflows/materialize-npm-nested-metadata-validation.yml, and .github/workflows/trigger-npm-nested-metadata-materializer.yml; do not add another branch writer, encoded patch, trigger, source snapshot, or temporary workflow.

Implement the narrow npm v2/v3 rule directly: a metadata-only nested node_modules location is accepted only when exactly one canonical root package has the same normalized scoped/unscoped identity and exact nonempty version, a valid https://registry.npmjs.org/...tgz URL, and one valid SHA-512 SRI. Reject missing or ambiguous canonical metadata, version mismatch, partial pins, malformed identities, conflicting complete pins, non-registry origin, port/userinfo/query/fragment abuse, invalid integrity, unsafe path, or link. Consume the original lock unchanged; never repair or synthesize metadata.

Add the BandScope-shaped positive case and every stated negative case. Run Python 3.10/3.14, focused and complete central tests, 100% production statement/branch/public-docstring coverage, Ruff, compileall, workflow contract, security and supply-chain checks, and git diff --check. Commit normally; no amend, force-push, Ready transition, approval, merge, release, reviewer-credential change, or protection change. Stop without mutation if the live head differs before writing.

Comment thread .github/workflows/pr807-apply-nested-metadata-once.yml Fixed

Copy link
Copy Markdown
Contributor Author

/oc

Fix the current exact-head failure directly in permanent source; do not rely on, add, or retain a one-shot/branch-writer workflow.

The Python 3.14 quality run proves 12 focused failures because scripts/ci/materialize_base_javascript_packages.py still executes the old rule that every non-link node_modules location must carry local resolved and integrity. Apply the reviewed logic currently represented by scripts/ci/apply_pr807_nested_metadata.py:

  • derive an exact scoped or unscoped package identity after the final node_modules segment;
  • keep complete local pins subject to the existing HTTPS npm-registry URL and SHA-512 SRI checks;
  • reject a partial resolved/integrity pair;
  • allow a metadata-only nested location only with a nonempty exact version and one canonical node_modules/<identity> non-link entry whose exact version matches and whose complete registry/SRI pin validates;
  • reject metadata-only canonical-root entries, malformed identities, missing/linked canonical entries, mismatched versions, hostile origins, and invalid integrity;
  • preserve lockfile-v2/v3, path, traversal, workspace-link, userinfo, port, query, fragment, tarball, and unchanged-lock-byte boundaries.

Add the APA 7 doctoring and CHANGELOG entry, remove scripts/ci/apply_pr807_nested_metadata.py and .github/workflows/pr807-apply-nested-metadata-once.yml from the final tree, and keep only the permanent quality workflow. Run the two focused suites with 100% branch coverage, production docstrings, Python 3.10/3.14 compilation, complete central tests/coverage, and git diff --check. Keep Draft until the workflow-free exact head is green.

Copy link
Copy Markdown
Contributor Author

@opencode-agent address

The branch has been restored to a reviewable TDD state at exact head 0ce11b73992851afe991a5e86991db398c9d9900: every PR-controlled writer, trigger, one-shot materializer, and patch applicator has been deleted. Do not recreate any source-writing workflow.

Implement the permanent change directly in scripts/ci/materialize_base_javascript_packages.py through an ordinary reviewed commit. Preserve every existing v2/v3 path, workspace-link, HTTPS public-registry, port/userinfo/query/fragment, SHA-512 SRI, and unchanged-lock boundary. Required contract:

  • parse each non-link node_modules location into one exact scoped or unscoped package identity from the suffix after its last node_modules component;
  • a complete location must still validate its own resolved and integrity exactly as today;
  • a metadata-only location may omit both fields only when it has a nonempty exact string version and one canonical root key node_modules/<identity> has the same version plus a complete valid public-registry tarball and SHA-512 pin;
  • reject canonical root entries that are themselves metadata-only, missing, malformed, version-mismatched, or partial;
  • reject any location declaring exactly one of resolved/integrity as must not partially declare;
  • preserve independently complete nested pins even when their version differs from the canonical root;
  • reject malformed scoped identities such as @scope without a package and extra components after a scoped package;
  • consume the lock unchanged; do not repair or synthesize fields.

Keep tests/test_npm_nested_metadata_lock_validation.py authoritative, add complete statement/branch/docstring coverage in the existing permanent quality workflow, update APA 7 doctoring and CHANGELOG.md through ordinary commits, then rerun every exact-head quality/security/supply-chain gate. Keep Draft and merge-block labels until GREEN and independent approval.

Copy link
Copy Markdown
Contributor Author

@opencode-agent address

Implement the permanent production change on exact current Draft head 0ce11b73992851afe991a5e86991db398c9d9900. Quality run 31139010086 proves Python 3.10 compilation and every security/supply-chain gate are green, while the Python 3.14 focused job has 12 RED failures because the branch currently contains only the quality workflow and tests; scripts/ci/materialize_base_javascript_packages.py is unchanged.

Apply the minimal two-pass validator:

  1. Parse and validate every package location/path and workspace link exactly as today.
  2. Derive the npm package identity from the suffix after the last node_modules component:
    • unscoped identity: exactly one nonempty segment;
    • scoped identity: exactly two nonempty segments, first beginning with @;
    • reject incomplete or extra-segment identities as malformed npm package identity.
  3. Classify each non-link location:
    • complete pin: version, resolved, and integrity all present; validate nonempty version, exact public npm HTTPS tarball provenance, no port/userinfo/query/fragment, and one SHA-512 SRI;
    • metadata-only: none of resolved/integrity present; require a nonempty exact version;
    • partial pin: exactly one of resolved/integrity present; fail with must not partially declare.
  4. Build canonical pins only from complete root locations of the exact form node_modules/<identity>—not nested copies. Preserve independently complete nested pins as independently validated artifacts.
  5. For each metadata-only location, require exactly one safe canonical root pin with the same package identity and exact version. Fail separately for missing canonical root pin and version mismatch.
  6. Do not repair, synthesize, reorder, or rewrite the lockfile; validation only.

Add scripts/ci/materialize_base_javascript_packages.py, APA 7 doctoring, and CHANGELOG.md to the permanent PR scope and quality-workflow triggers. Keep the branch workflow-free except the permanent read-only quality gate; no self-removing or write-capable patcher may return.

Run:

python -m coverage erase
python -m coverage run --branch -m pytest -q \
  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
python -m interrogate scripts/ci
python -m compileall -q scripts/ci tests
pytest -q

Then rerun exact-head CodeQL, Python Security, SAST, Security Scan, Secret Scan, OSV, Scorecard, SBOM, CodeRabbit/OpenCode/Noema, and keep Draft until all evidence is current and a qualifying independent approval exists.

Copy link
Copy Markdown
Contributor Author

@opencode-agent address

Exact head 0ce11b73992851afe991a5e86991db398c9d9900 has Python 3.10 compatibility and every security/supply-chain gate green. Python 3.14 focused quality is intentionally RED: 44 passed, 12 failed, all because validate_head_npm_lock() still requires every non-link node_modules location to repeat resolved and integrity.

Implement the minimum permanent production correction directly in scripts/ci/materialize_base_javascript_packages.py; do not reintroduce a PR-controlled source-rewriting workflow.

  • Derive package identity from the final node_modules segment. An unscoped identity is exactly one following segment; a scoped identity is exactly @scope/name. Reject an empty scope/name, @scope without a package, or any extra path segment after the identity as malformed npm package identity.
  • Classify artifact fields exactly: both resolved and integrity = complete pin; neither = metadata-only; exactly one = fail with must not partially declare.
  • Continue validating every complete pin with the existing closed public npm-registry HTTPS/tarball/SHA-512 contract.
  • Build canonical root pins only from exact top-level paths node_modules/name and node_modules/@scope/name. A root canonical entry must be a complete validated pin; a metadata-only root entry fails with canonical root pin.
  • A metadata-only nested location must have one nonempty exact string version, resolve to a complete validated canonical root pin with the same package identity, and match its exact version. Otherwise fail with the committed canonical root pin, nonempty exact version, or exact canonical version contract.
  • A fully and independently pinned nested package remains valid even when a canonical root package with the same identity has a different version. Never synthesize, repair, or mutate lock metadata.
  • Preserve link/path safety, lockfile v2/v3, registry URL, port/userinfo/query/fragment, tarball identity, integrity, bounded parsing, and fail-closed behavior.

Add the permanent production tests to the authoritative existing module, complete public docstrings, APA 7 doctoring and CHANGELOG.md; update the stale PR body so it no longer claims a self-removing workflow. Run focused 100% production statement/branch/docstring coverage, Python 3.10/3.14 compilation, the complete central suite, actionlint, and all security/supply-chain gates on one unchanged head. Keep Draft until GREEN.

Copy link
Copy Markdown
Contributor Author

@opencode-agent address

Implement the permanent GREEN production change for exact head 0ce11b73992851afe991a5e86991db398c9d9900 directly through ordinary reviewed commits. Do not add a trigger, materializer, repair, self-removing, encoded-patch, or branch-writing workflow.

Update scripts/ci/materialize_base_javascript_packages.py::validate_head_npm_lock test-first so npm v2/v3 metadata-only nested locations are accepted only through one exact canonical root pin:

  1. Parse and validate every node_modules path first. Derive the package identity from the segments after the final node_modules: exactly one unscoped segment or exactly @scope/name; reject incomplete scopes and extra segments as malformed npm package identity.
  2. Require every non-link package entry to have a nonempty string version.
  3. Classify resolved/integrity as both absent, both present, or partial. Reject partial declarations as must not partially declare.
  4. Validate every complete pin with the existing exact HTTPS registry URL, port/userinfo/query/fragment/tarball, and SHA-512 SRI controls.
  5. A root entry at exactly node_modules/<identity> must be a complete pin; a metadata-only root entry fails with canonical root pin.
  6. A metadata-only nested entry may inherit only the complete canonical root pin for the same identity and exact version. Reject missing root pins and version mismatches with the permanent contract messages.
  7. Independently complete nested pins remain valid, including a different nested version.
  8. Preserve link handling, path safety, lockfile v2/v3 constraints, and all pre-existing tests.

Then add authoritative npm v11 package-lock doctoring with APA 7 primary-source references and CHANGELOG.md, include implementation/docs/changelog in the permanent quality workflow path filters, run focused tests with 100% production statement/branch coverage and docstrings, run the complete central suite, compile, remove all transient scope, and leave the PR Draft until the workflow-free exact head is green.

Copy link
Copy Markdown
Contributor Author

@opencode-agent address

Continue from exact current head 0d7b3e6251441cb0f33ee7675ae7f6d9e81fd047; stop without writing if the live head differs.

A second RED contract set now closes the remaining descriptor ancestry boundary in tests/test_javascript_materializer_descriptor_ancestry.py. The current implementation is still expected to fail because it opens an absolute parent path after pathname checks, does not bind a newly created nested directory to its original inode, does not fsync published directories, does not fail before output creation when dir_fd capabilities are unavailable, and leaves partial owned output after a failed write.

Implement the smallest coherent root fix, not test exceptions:

  • walk every existing output ancestor from a held filesystem-root or trusted-anchor descriptor; never open candidate.parent through an absolute pathname after validation;
  • when creating a child directory, bind the opened descriptor to the exact inode created by this invocation and reject replacement before use;
  • synchronize file bytes and every directory whose entries are published, including nested, project, and output directories;
  • explicitly verify required dir_fd, no-follow, directory-open, exclusive-create, stat-without-follow, write, fsync, and unlink/rmdir capabilities before creating the output tree; fail closed with a bounded error;
  • maintain an ownership journal of newly created files/directories and remove only those entries, descriptor-relatively and deepest-first, when materialization fails; preserve all pre-existing operator content;
  • retain the earlier root-binding, hard-link, destination-swap, stalled-write, lexical-path, canonical-pin, coverage, and docstring contracts.

Update doctoring and CHANGELOG to state the full descriptor-anchored ancestry, durability, capability, and cleanup boundary with APA 7 POSIX.1-2024 and CWE-59/CWE-367 references. Run the expanded focused suite RED→GREEN, then 100% statement/branch coverage, interrogate 100%, Python 3.10 compile, full central tests, and git diff --check. Commit normally on this existing branch only; no new branch, PR, workflow, helper, patch artifact, or bypass.

Copy link
Copy Markdown
Contributor Author

@opencode-agent

Continue on exact current head 0d7b3e6251441cb0f33ee7675ae7f6d9e81fd047 and fix the five intended RED descriptor-ancestry failures from npm Nested Metadata Validation Quality CI run 31157314799, job 92799545489. Do not weaken or delete tests/test_javascript_materializer_descriptor_ancestry.py.

Root-cause repair requirements:

  1. Replace any absolute multi-component output-parent open with a held-descriptor walk from filesystem root. Capture each existing ancestor's no-follow identity, open each component relative to its held parent with O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC, and verify descriptor/entry identity so rename-to-symlink or ancestor rebinding fails with bounded ValueError containing ancestor/symlink/changed semantics.
  2. When creating a new nested directory, record the no-follow entry identity immediately after mkdir, open it relative to the held parent, then compare fstat to the recorded entry. Reject replacement between creation and open with a bounded directory-binding/inode diagnostic.
  3. fsync every generated file and every directory whose entries are created or removed: nested parent, project directory, output directory, and relevant ancestor/parent after publication or rollback. Preserve file-before-directory ordering.
  4. Before creating any output path, fail closed unless the runtime exposes all required descriptor-relative/no-follow capabilities. Validate os.supports_dir_fd/os.supports_follow_symlinks and required flags/functions; use the exact bounded diagnostic family asserted by the tests.
  5. Track only entries created by the current invocation. On any failure, recursively remove owned partial project directories/files through held descriptors, fsync affected directories, and preserve all pre-existing operator entries. Never path-walk attacker-controlled replacement trees during cleanup.
  6. Retain the existing 80 passing focused tests, exact canonical npm-pin behavior, Python 3.10 compatibility, public schema, no network/repair behavior, and 100% statement/branch/docstring gates. Add only tests needed for new production branches.
  7. Update docs/doctoring/npm-nested-metadata-canonical-pins.md and CHANGELOG.md with the completed ancestry pinning, durability, capability, and rollback boundary plus POSIX.1-2024 and MITRE CWE-59/CWE-367 APA 7 references.
  8. Run the focused suite until all descriptor tests pass and production reports 100% statements/branches, then the complete central suite, compilation, docstrings, clean diff, and exact-head workflows.

Update this branch only. Do not create another branch/PR, temporary workflow, patch artifact, writer automation, force push, approval bypass, merge, or release.

Copy link
Copy Markdown
Contributor Author

@opencode-agent address

Fix exact current head e584fba4f9b191451fe6c451d3d436d23a21133f; stop without writing if the live head differs.

Quality run 31158550711 fails before exercising the secure writer because _require_descriptor_relative_capabilities() compares the currently monkeypatched os.open object against os.supports_dir_fd, whose set contains the original CPython builtin object. Four adversarial tests replace os.open to trigger races or bounded failures, so the preflight incorrectly reports the platform unsupported.

Apply the minimal root fix: capture the required CPython filesystem callables once at module import (before test monkeypatching) and have the capability preflight compare those immutable original callables against os.supports_dir_fd. Keep actual filesystem operations routed through the live os.* attributes so race injections still exercise production branches. Preserve the test that monkeypatches os.supports_dir_fd to an empty set; it must continue to fail closed before any output path is created. Also require os.stat in os.supports_follow_symlinks because the implementation depends on follow_symlinks=False.

Rerun the exact expanded focused suite. Do not weaken regex/assertions merely to bypass the preflight. After these four tests reach the intended branches, address any remaining ancestor, inode, durability, or ownership-cleanup failures at their source, then restore 100% statement/branch coverage, interrogate 100%, Python 3.10 compilation, full central tests, and git diff --check. Commit normally on this branch only; no new branch, PR, workflow, helper, patch artifact, or bypass.

Copy link
Copy Markdown
Contributor Author

@opencode-agent address

Continue from exact current head 6bbc93a15470203d303fc1ea9ed3ef2e2adf5026; stop without writing if the live head differs.

Do not retain the new tests/conftest.py _DynamicDirectoryFdSupport autouse workaround. It changes global platform capability evidence for selected tests and masks the production defect. Implement the immutable-original-callable capability check in the production module, remove that conftest instrumentation entirely, and keep live os.* calls for adversarial injection.

A new RED suite, tests/test_javascript_materializer_creation_cleanup_security.py, adds four non-negotiable contracts:

  1. a forwarding os.open wrapper must not make a supported platform appear unsupported;
  2. missing os.stat(..., follow_symlinks=False) capability must fail before creating output;
  3. replacing a previously trusted ancestor immediately after a missing parent is created must not create generated_locks through an attacker symlink; and
  4. failure while writing the second generated file must roll back every file/directory owned by this invocation while preserving pre-existing operator content.

The current pathname-based candidate.parent.mkdir(parents=True) / candidate.mkdir() and single-file cleanup are expected to fail the last two. Replace them with descriptor-anchored component traversal/creation from a held trusted anchor and a complete descriptor-relative ownership journal, deepest-first rollback, and directory fsync. Do not merely broaden exception regexes or preserve partial forensic outputs—the materialized cache is acceptance evidence and must be atomic.

Add both tests/test_javascript_materializer_creation_cleanup_security.py and tests/test_javascript_materializer_output_edge_coverage.py to workflow path triggers, Python 3.10 compilation, and the focused 100% branch-coverage command. Update doctoring and CHANGELOG for atomic publication/rollback. Run expanded RED→GREEN, interrogate 100%, full central tests at 100%, and git diff --check. Existing branch only; no new PR, helper, patch artifact, classifier, or bypass.

Copy link
Copy Markdown
Contributor Author

/oc Refetch the exact live head of this PR before editing and abort if it moved. Diagnose and fix the four same-head failures from npm Nested Metadata Validation Quality CI run 31160190475, job 92808561033, without weakening descriptor-relative/no-follow publication. Required source fixes in scripts/ci/materialize_base_javascript_packages.py: (1) capability detection must tolerate a transparent monkeypatched forwarding os.open by testing the immutable platform-supported callable set while still failing closed when real dir_fd support is absent; (2) require os.stat membership in os.supports_follow_symlinks before any mutation; (3) remove pathname-based Path.mkdir(parents=True) creation and create every missing output ancestor component through held directory FDs with O_DIRECTORY|O_NOFOLLOW, inode revalidation, and parent fsync so an ancestor swap cannot create generated_locks through an attacker symlink; (4) make project publication transactional—on any later write failure, remove every earlier generated file and generated nested/project directory that is still owned by its recorded dev/inode, while preserving pre-existing operator files and any replacement inode. Update/add focused tests first, including the existing tests/test_javascript_materializer_creation_cleanup_security.py contracts, run the complete focused 100% statement/branch/docstring gate and the full central test suite, and commit only this branch if all pass. Do not add a one-shot workflow, patch artifact, alternate writer, or relax O_EXCL, no-follow, single-link, fsync, exact-head, or authenticated-lock boundaries.

Copy link
Copy Markdown
Contributor Author

/oc Fix only the fresh exact-head quality regression on f60beed707aef04c81bdfb1566acdd989b40eb9; stop without writing if the live head differs. The failing run is 31161622367, job 92813033566: four descriptor-race tests now fail at _require_descriptor_relative_capabilities() because they instrument os.open, while os.supports_dir_fd records the original built-in callable identity. Do not restore the removed capability-masking test fixture and do not weaken the fail-closed unsupported-runtime contract.

Apply the smallest production fix test-first: bind the original descriptor-capable OS callables at module import (for example an immutable _REQUIRED_DIR_FD_FUNCTIONS = (os.open, os.mkdir, os.stat, os.unlink, os.rmdir)) and have _require_descriptor_relative_capabilities() compare the live os.supports_dir_fd capability set against those original callables. This must allow race instrumentation that wraps os.open while still failing when os.supports_dir_fd itself lacks a required original primitive. Preserve O_DIRECTORY/O_NOFOLLOW checks, all descriptor-relative/no-follow/inode/link/fsync protections, canonical npm validation, exact authenticated Git identities, current permissions, and all existing security behavior.

Verify RED→GREEN with the exact four failing regressions plus test_materializer_fails_closed_without_descriptor_relative_capabilities, then run the full permanent quality workflow with 100% production statement/branch coverage, 100% public docstrings, compileall, and git diff --check. Commit normally on this existing branch. Do not create another branch, PR, workflow, patch artifact/fragment, classifier, or bypass; do not merge or mark Ready.

Copy link
Copy Markdown
Contributor Author

/oc Continue only from exact live head f3ef0af11863255aee1ad8b875676d656ab99a68; abort without writing if the head moved. The first same-head repair is verified in focused scope: npm Nested Metadata Validation Quality CI run 31163640520 passed Python 3.10, then passed all 101 focused tests with 439/439 statements and 178/178 branches plus 100% public docstrings. The complete central regression then exposed exactly three pre-existing RED security contracts in tests/test_javascript_materializer_creation_cleanup_security.py (1022 passed, 3 failed):

  1. test_materializer_rejects_missing_follow_symlink_capability_before_mutation: _require_descriptor_relative_capabilities() must also require the original os.stat identity in the live os.supports_follow_symlinks set before any mutation. Preserve the immutable-callable approach already added for supports_dir_fd so transparent instrumentation does not falsify platform capability.
  2. test_missing_ancestor_swap_never_creates_output_through_attacker_symlink: remove pathname-based recursive output creation (Path.mkdir(parents=True) / equivalent). Create and bind missing output ancestors through held directory FDs from an already opened trusted ancestor/root, using O_DIRECTORY|O_NOFOLLOW, descriptor-relative mkdir/stat/open, dev+inode revalidation and parent fsync. If the existing RED test instruments the pathname API that the secure implementation intentionally removes, refine the test to instrument the descriptor-relative mkdir boundary while preserving the same adversarial swap and proving no attacker-tree output is created.
  3. test_late_write_failure_rolls_back_every_owned_file_and_directory: make publication transactional. On any later write failure, remove all earlier generated files and generated nested/project directories only while their recorded dev/inode identities are still published; preserve pre-existing operator entries and any attacker replacement inode. Use descriptor-relative cleanup and fsync; do not recursively delete unowned pathname trees.

Use these observed same-head failures as RED. Preserve exact authenticated base/head lock identity, npm canonical-pin validation, O_EXCL, no-follow, single-link, bounded write progress, file/path descriptor identity checks, fsync, fail-closed unsupported-runtime behavior, and all current permissions. Run the three failing tests plus forwarding-open and missing-dir-fd capability tests first, then all focused materializer tests with 100% statement/branch/docstrings, then the complete central suite, compileall and git diff --check. Commit normally on this existing branch only. Do not create another branch/PR/workflow, patch artifact/fragment, temporary writer, classifier, or bypass; do not mark Ready or merge.

Copy link
Copy Markdown
Contributor Author

@opencode-agent Implement the minimal permanent GREEN repair directly on exact current head f3ef0af11863255aee1ad8b875676d656ab99a68 of Draft branch fix/npm-nested-metadata-lock-validation.

Exact-head quality run 31163640520, job 92819403826, proves the focused 101-test descriptor suite and 439 statements/178 branches at 100%, but the complete central suite fails exactly three security regressions in tests/test_javascript_materializer_creation_cleanup_security.py:

  1. test_materializer_rejects_missing_follow_symlink_capability_before_mutation: clearing os.supports_follow_symlinks does not fail before output mutation.
  2. test_missing_ancestor_swap_never_creates_output_through_attacker_symlink: pathname-based missing-ancestor creation can be redirected after a checked parent is swapped, and creates attacker_root/.../generated_locks.
  3. test_late_write_failure_rolls_back_every_owned_file_and_directory: a zero-progress second write leaves owned project-000 behind instead of restoring the pre-run directory while preserving operator-note.txt.

Repair test-first without weakening the canonical npm-pin validation or portability boundary:

  • require every descriptor-relative/no-follow capability actually used, including os.stat(..., follow_symlinks=False) support, before creating any output component;
  • replace pathname-based creation of missing ancestors/output/project directories with a descriptor-anchored walk from a validated existing ancestor using dir_fd, O_DIRECTORY, O_NOFOLLOW, close-on-exec where available, and exact post-open identity checks;
  • never follow a symlink introduced between component checks; never create anything through the attacker path;
  • track only entries created by this invocation and roll back owned files/directories in reverse dependency order after any failure, preserving pre-existing operator data and refusing ambiguous cleanup;
  • retain forward-progress-checked writes, fsync, path/descriptor identity and single-link revalidation, lexical relative paths, stable non-leaking errors, and fail closed when primitives are unavailable;
  • add tests/test_javascript_materializer_creation_cleanup_security.py to the focused quality workflow and compilation list so this boundary cannot be omitted again;
  • keep 100% production statement/branch/docstrings, Python 3.10/3.14 compatibility, exact-head checkout, immutable actions, APA 7 doctoring, and [Unreleased] evidence.

Do not add a temporary/self-removing/encoded-patch/branch-writer workflow, change reviewer credentials, weaken a security result, merge, mark Ready, rebase, retarget, release, or publish. Run the three failing tests first, then every focused materializer test, the complete central suite, coverage/docstrings/compileall/git diff --check, and exact-head security/supply-chain workflows.

Copy link
Copy Markdown
Contributor Author

@opencode-agent Repair exact current head f3ef0af11863255aee1ad8b875676d656ab99a68 directly on this branch. The focused npm-lock slice is GREEN at 101 tests and 100% production statement/branch/docstring coverage; the complete central suite exposes three valid descriptor-relative materializer defects in tests/test_javascript_materializer_creation_cleanup_security.py.

Implement the smallest authoritative-source fix in scripts/ci/materialize_base_javascript_packages.py, test-first, without opening a stacked PR or adding any branch-writer/repair workflow:

  1. Immutable capability identity + follow-symlink support. Capture the original CPython os.open, os.mkdir, os.stat, os.unlink, and os.rmdir callables at import time for capability membership checks, so a forwarding instrumentation wrapper around os.open does not change platform capability. Require the original os.stat callable to be present in os.supports_follow_symlinks as well as all original callables in os.supports_dir_fd; otherwise fail before creating any output path with the existing descriptor-relative-unavailable error.

  2. Descriptor-relative missing-ancestor creation. Remove the pathname-based Path.mkdir(parents=True) / subsequent pathname reopen sequence from _open_output_directory. Locate and open the nearest existing non-symlink directory ancestor once using O_DIRECTORY|O_NOFOLLOW, then create and open each missing component one at a time with dir_fd, follow_symlinks=False, inode checks, and fsync. Never re-resolve a newly created descendant through the original pathname. Keep the final full-path binding verification so an ancestor replacement is detected, but ensure the attack in test_missing_ancestor_swap_never_creates_output_through_attacker_symlink cannot create anything under the attacker target.

  3. Complete owned-project rollback. A fresh project-NNN directory is wholly owned by the current materialization attempt. Track every created file and nested directory with descriptor-relative identity, and on any later write/fsync/validation failure remove owned files and directories in reverse order before removing the project directory. Preserve operator-owned entries already present in the output root. The os.write no-progress regression must leave only operator-note.txt.

Preserve all current npm canonical-pin validation, exact base/HEAD blob binding, no-follow/exclusive file creation, single-link checks, post-write inode checks, 100% coverage/docstrings, and Python 3.10 compatibility. Run the three failing security tests first, then the focused 101-test suite, then the complete central regression/coverage/docstring/compile/security suite. Keep Draft until the new exact head is fully GREEN.

Copy link
Copy Markdown
Contributor Author

@opencode-agent address

Repair exact head f3ef0af11863255aee1ad8b875676d656ab99a68 directly and keep the PR Draft. The focused 101-test materializer slice and 100% production statement/branch/docstring gates pass, but full run 31163640520 exposes three valid fail-closed regressions:

  1. _require_descriptor_relative_capabilities() must also require immutable os.stat membership in os.supports_follow_symlinks, while retaining immutable callable identities so test instrumentation of os.open does not falsify platform capability.
  2. _open_output_directory() must never create missing ancestors through unresolved pathnames after inspection. Create/bind missing components descriptor-relatively from a pinned existing ancestor; the trusted_root -> attacker_root swap test must raise before attacker_root/missing_parent/generated_locks is created.
  3. A late write failure must roll back every file and directory created by the current materialization attempt while preserving pre-existing operator entries. Cleanup must be identity-bound, descriptor-relative, no-follow, and refuse to remove replaced/foreign objects.

Use the existing failing tests as RED evidence, add only regressions needed for uncovered cleanup/race branches, preserve the npm canonical-pin contract and authenticated lock consumption, then run the complete exact-head suite. Do not add a one-shot or branch-writing repair workflow.

Copy link
Copy Markdown
Contributor Author

/oc Refetch the live PR head and stop without writing unless it is exactly f3ef0af11863255aee1ad8b875676d656ab99a68. Exact-head npm Nested Metadata Validation Quality CI run 31163640520, job 92831568748, proves the focused materializer slice is sound (101 focused tests; materialize_base_javascript_packages.py 439/439 statements and 178/178 branches; public docstrings 100%; Python 3.10 compatibility green), but the complete central suite has three valid fail-closed filesystem regressions. Fix production, not the assertions.

  1. Capability contract. _require_descriptor_relative_capabilities() currently checks only os.supports_dir_fd. It must also fail before any mutation unless the immutable CPython os.stat callable used by this module is present in os.supports_follow_symlinks, while preserving the existing contract that harmless monkeypatch instrumentation of os.open does not redefine platform capability. Keep semantic capability membership separate from mutable wrapper identity.
  2. Missing-ancestor creation. _open_output_directory() still creates candidate.parent and candidate through pathname-based Path.mkdir, leaving a TOCTOU window. Create and open every missing output component from a pinned, no-follow directory descriptor; compare pre-open and post-open device/inode identity, synchronize each published directory entry, and bind the final pathname back to the held output descriptor. An ancestor rename/symlink swap must fail without creating generated_locks in the attacker tree. Update the adversarial test instrumentation only as needed to trigger the same semantic race through the descriptor-relative implementation; do not weaken the assertion.
  3. Transactional rollback. A late or stalled file write currently removes only the in-progress file and then attempts to remove a nonempty project-000, leaving previously written files/directories behind. Treat each fresh project directory as one transaction-owned namespace. On any project failure, remove every generated regular file and nested empty directory in reverse order through held/pinned descriptors (never follow symlinks), verify the expected inode before deletion, then remove the project directory. Preserve every pre-existing output-root entry such as operator-note.txt; never recursively delete unowned output-root content.

Run the three exact failing tests first, add a nested rollback adversarial case if the implementation introduces nested transaction entries, then run the focused 101-test contract, the complete central suite, production statement/branch coverage 100%, public docstrings 100%, Python 3.10 compilation, compileall, and git diff --check. Keep the PR Draft until the new exact head is green. Do not add a temporary/one-shot/self-modifying workflow, branch writer, encoded patch, credential/provider/reviewer change, approval, merge, release, or force push.

Copy link
Copy Markdown
Contributor Author

/oc Refetch the exact live PR head and abort unless it is f3ef0af11863255aee1ad8b875676d656ab99a68. Fix npm Nested Metadata Validation Quality CI run 31163640520 directly on this branch without weakening the focused nested-metadata implementation. The complete suite exposes three fail-closed output-materialization regressions in scripts/ci/materialize_base_javascript_packages.py: (1) missing no-follow/descriptor-relative symlink capability no longer fails before any output mutation; (2) a missing-ancestor pathname race can create generated_locks through an attacker-swapped symlink; (3) late write failure leaves an owned project-000 directory instead of rolling back all materializer-owned entries while preserving operator files. Keep the existing RED tests unchanged. Restore a descriptor-anchored, no-follow creation path that validates required dir_fd/follow-symlink capabilities before mutation, creates each missing ancestor and output entry relative to already-open trusted directory descriptors, detects identity changes, and records every owned file/directory for reverse-order rollback on any failure. Never follow attacker-controlled pathnames after capability validation, never remove pre-existing operator data, and do not suppress capability/race/write errors. Run the three failing tests first, then the complete central suite with 100% statement/branch coverage, docstrings, compileall, and exact-head security checks. Keep Draft until the new head is green; do not create a temporary/branch-writer workflow.

Copy link
Copy Markdown
Contributor Author

@opencode-agent Repair the exact current Draft head f3ef0af11863255aee1ad8b875676d656ab99a68 directly in permanent source; do not add temporary/self-removing/encoded-patch/branch-writing workflows.

Exact-head run 31163640520 proves the focused npm canonical-pin suite and 100% target coverage pass, but the complete central suite has three real output-materialization regressions:

  1. test_materializer_rejects_missing_follow_symlink_capability_before_mutation: fail closed before creating any path when mandatory descriptor-relative/no-follow stat capability is unavailable. Do not substitute callable identity checks for the platform capability set.
  2. test_missing_ancestor_swap_never_creates_output_through_attacker_symlink: never create the output through a pathname ancestor replaced with an attacker symlink. Create/verify missing ancestors through pinned directory descriptors with no-follow semantics, or refuse the operation before mutation.
  3. test_late_write_failure_rolls_back_every_owned_file_and_directory: on a late zero-progress/write failure, remove every materializer-owned file and directory created by this invocation while preserving pre-existing operator data.

Keep the canonical nested npm metadata behavior unchanged. Add/adjust test-first contracts only as needed, retain Python 3.10 compatibility, exact-head checkout, 100% production statement/branch/docstring coverage, compileall, security boundaries, authenticated-base inputs, and clean-tree verification. Update doctoring/CHANGELOG only for materially changed behavior. Keep Draft and do not approve, merge, release, or alter reviewer credentials/protection.

Copy link
Copy Markdown
Contributor Author

@opencode-agent address
@cwl-noema-review

Repair exact current head f3ef0af11863255aee1ad8b875676d656ab99a68 only; do not merge, retarget, or weaken tests. The dedicated quality run 31163640520 / job 92831568748 proved the focused npm-lock behavior GREEN (101 focused tests; 439/439 statements and 178/178 branches) but the complete central suite found three real materializer boundary failures:

  1. _require_descriptor_relative_capabilities() validates supports_dir_fd but not os.stat in os.supports_follow_symlinks, so the mandatory no-follow stat capability can be absent without a pre-mutation failure.
  2. _open_output_directory() creates a missing parent by pathname and then creates the output directory after an attacker can replace the trusted ancestor with a symlink; the exact regression observed attacker_root/missing_parent/generated_locks being created.
  3. A late second-file write failure removes the partial file but leaves the already-created project-000 directory and earlier generated entries, violating complete owned-output rollback while operator files must remain untouched.

Use TDD and the minimum durable source repair. Add the existing creation/cleanup security regressions to the focused exact-head quality workflow so the affected branches are measured at 100%; keep descriptor-relative/no-follow operations, immutable source identity, current npm canonical-pin validation, credentials, reviewer identities, and branch protections unchanged. Rerun the complete central suite, 100% production statement/branch coverage, 100% docstrings, Python 3.10/3.14 compilation, all security/supply-chain checks, and fresh current-head reviews. No one-shot or branch-writing repair workflow.

Copy link
Copy Markdown
Contributor Author

@coderabbitai autofix

Commit directly to exact head f3ef0af11863255aee1ad8b875676d656ab99a68 on this PR branch. Address only the three current complete-suite materializer security failures: immutable original os.* capability checks including os.stat follow-symlink support; descriptor-relative creation of missing output ancestors without pathname re-resolution; and reverse-order cleanup of all files/directories owned by a failed fresh project-NNN attempt while preserving operator-owned root entries. Preserve canonical npm pin validation, blob binding, no-follow/exclusive creation, inode/link checks, Python compatibility, 100% coverage/docstrings, and no temporary workflow. Do not open another PR.

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Autofix skipped. No unresolved CodeRabbit review comments with fix instructions found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants