Skip to content

fix(sync): add trusted Vitest verification adapter#1993

Open
gltanaka wants to merge 14 commits into
codex/issue-1932-jest-adapterfrom
codex/issue-1932-vitest-adapter
Open

fix(sync): add trusted Vitest verification adapter#1993
gltanaka wants to merge 14 commits into
codex/issue-1932-jest-adapterfrom
codex/issue-1932-vitest-adapter

Conversation

@gltanaka

Copy link
Copy Markdown
Contributor

Summary

  • Add a distinct trusted validator_id=vitest runner adapter.
  • Bind static Vitest config, setup and transform support closure; reject dynamic config, plugin/workspace/project controls, filtering/sharding/watch controls, and dirty executable modules.
  • Derive protected-base identities independently and execute exact protected paths via the local Vitest binary with a sanitized environment.

Test plan

  • conda run -n pdd pytest -q tests/test_sync_core_runner.py tests/test_sync_core_runner_jest.py tests/test_sync_core_runner_vitest.py
  • conda run -n pdd pylint pdd/sync_core/runner.py tests/test_sync_core_runner_vitest.py
  • Real fixture through run_profile using node_modules/vitest/vitest.mjs: PASS 1 protected Vitest tests passed
  • Wheel/source parity SHA-256 check for pdd/sync_core/runner.py

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@gltanaka

Copy link
Copy Markdown
Contributor Author

Pre-fix investigation

  • Base verified: origin/codex/issue-1932-jest-adapter at dc6d5ccd8 (fix(sync): fail closed for unbound Jest project plugins).
  • History mined: dc74dd60c / 98e5e9e2b / dc6d5ccd8 establish the trusted JS runner convention: static config only, protected-base identity collection, direct local binary argv, sanitized environment, and fail-closed closure controls.
  • Sibling hunt: no trusted Vitest adapter existed. The only related trusted-runner implementation is Jest; this PR deliberately adds a separate vitest adapter and does not alter pytest or Jest behavior.
  • Scope decision: pdd/sync_core/runner.py plus dedicated tests/test_sync_core_runner_vitest.py only. No aliases, transaction/certificate/lifecycle work, includes/preprocess/CI changes, or .pdd files.

@gltanaka

Copy link
Copy Markdown
Contributor Author

TDD evidence: red then green

Red commit: fafcd298e test(sync): reproduce missing trusted Vitest adapter

Command:

conda run -n pdd pytest -q tests/test_sync_core_runner_vitest.py

Result before the fix: 16 failed; the first failure was TypeError: RunnerConfig.__init__() got an unexpected keyword argument 'vitest_command', reproducing the absence of a trusted Vitest adapter surface. This commit was pushed before the implementation commit.

Green commit: d4b1ba671 fix(sync): add trusted Vitest verification adapter

Commands:

conda run -n pdd pytest -q tests/test_sync_core_runner.py tests/test_sync_core_runner_jest.py tests/test_sync_core_runner_vitest.py
conda run -n pdd pylint pdd/sync_core/runner.py tests/test_sync_core_runner_vitest.py

Result: both commands passed. The focused Vitest contract suite contains 20 passing cases covering pass/fail/skip/todo/zero/timeout/malformed output, identity mismatches, protected test/config/setup/transform mutation, dirty support, secret stripping, dynamic config, and unbound watch/shard/projects/plugins.

@gltanaka

Copy link
Copy Markdown
Contributor Author

E2E and parity evidence

Created a disposable local Git fixture with real vitest@4.0.18, a static vitest.config.json, and tests/widget.test.js. Invoked the production runner path through run_profile; it directly ran node_modules/vitest/vitest.mjs run against both the protected-base clone and checked head.

Result:

PASS 1 protected Vitest tests passed

Also built a wheel outside the repository and compared SHA-256 of source versus wheel pdd/sync_core/runner.py: wheel/source runner.py parity: PASS.

Final stack check: rebased against freshly fetched origin/codex/issue-1932-jest-adapter; it was already current at dc6d5ccd8, so no history rewrite or force-with-lease push was needed.

@gltanaka gltanaka force-pushed the codex/issue-1932-jest-adapter branch from dc6d5cc to 92dee25 Compare July 11, 2026 01:44
@gltanaka gltanaka force-pushed the codex/issue-1932-vitest-adapter branch from d4b1ba6 to 4fa0332 Compare July 11, 2026 01:45
@gltanaka

Copy link
Copy Markdown
Contributor Author

Refreshed stacked-base evidence

The Jest parent was refreshed by the stack maintainer. I fetched it and rebased this PR onto exact parent 92dee253589e4a071c6e0038a8c0af8330394dfa before rerunning verification.

Rewritten stacked commits now on the PR:

  • 360f48213 test(sync): reproduce missing trusted Vitest adapter
  • 4fa03324c fix(sync): add trusted Vitest verification adapter

Post-rebase verification passed:

conda run -n pdd pytest -q tests/test_sync_core_runner.py tests/test_sync_core_runner_jest.py tests/test_sync_core_runner_vitest.py
conda run -n pdd pylint pdd/sync_core/runner.py tests/test_sync_core_runner_vitest.py

The real local vitest@4.0.18 fixture again returned PASS 1 protected Vitest tests passed; wheel/source runner.py SHA-256 parity also passed. Because rebase rewrote the child commits, the verified branch was updated with git push --force-with-lease only.

Final state: clean worktree, no unpushed commits, PR base remains codex/issue-1932-jest-adapter.

@gltanaka gltanaka force-pushed the codex/issue-1932-jest-adapter branch from 92dee25 to 39e2690 Compare July 11, 2026 02:41
@gltanaka gltanaka force-pushed the codex/issue-1932-vitest-adapter branch from 4fa0332 to bbafa28 Compare July 11, 2026 02:42
@gltanaka

Copy link
Copy Markdown
Contributor Author

Codex xhigh adversarial review — PR #1993 @ bbafa28

Verdict: DON'T MERGE
Findings: 2 total (2 P1, 0 P2)

P1 (blocking)

  • [P1] Vitest closure drops protected-test imports — pdd/sync_core/runner.py:439-471,1327-1357
    _vitest_support_closure explicitly discards test_paths, so the hashed/quarantined closure contains only the static config plus setup/transform support. run_obligation checks changes in artifact_paths | support_paths, but committed helpers imported by the protected test are absent from support_paths; a candidate can change tests/helper.ts (or a transitive local module) while leaving the protected test file and identities unchanged, and the head Vitest execution can still PASS with candidate-controlled helper behavior.
    Suggested fix: Start the Vitest support closure from the protected test paths, recursively include literal local imports from those tests and their helpers, and add a regression where a committed imported helper changes without touching the test file.
  • [P1] The Vitest adapter trusts an unbound candidate Node toolchain — pdd/sync_core/runner.py:790-900,1492-1518
    The default command resolves root/node_modules/vitest/vitest.mjs, while protected-base collection deliberately runs the base clone using command_root=root. That means the supposedly trusted collector/executor can come from ignored candidate-worktree package contents that are not part of the protected SHA, the closure digest, or the dirty support check. A modified local Vitest CLI can emit the expected JSON identities/statuses and make forged evidence look trusted.
    Suggested fix: Run Vitest only from a protected, pinned external toolchain whose digest/version is included in the runner identity, or bind and verify the full local package/tool closure before collection and execution.

P2 (non-blocking)

  • None.

Notes

  • Reviewed against actual base codex/issue-1932-jest-adapter @ 39e26901f4053e22ebdd297e9be5d2586b4c7ff6, not main.

@gltanaka

Copy link
Copy Markdown
Contributor Author

Implementer response to Codex xhigh review

@codex Re: #1993 (comment)

P1 #1: Vitest closure drops protected-test imports

Status: FIXING
Plan: Add failing coverage for a changed committed helper imported by a protected Vitest test, then include recursive local imports from protected tests in the bound support closure.

P1 #2: Vitest adapter trusts an unbound candidate Node toolchain

Status: FIXING
Plan: Add failing coverage for ignored local node_modules Vitest trust, then require the Vitest toolchain to be digest-bound/protected or reject it before accepting trusted evidence.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create an environment for this repo.

@gltanaka gltanaka force-pushed the codex/issue-1932-vitest-adapter branch from bbafa28 to 3bbe500 Compare July 11, 2026 05:53
@gltanaka

Copy link
Copy Markdown
Contributor Author

Implementer evidence

Fixed both xhigh P1 findings and rebased this stacked branch onto updated PR #1988.

Commits:

  • cb0df824a test: cover Vitest trust boundary gaps
  • 3bbe50030 fix: bind Vitest support closure and reject local runner

TDD red evidence before fix:

  • pytest -q tests/test_sync_core_runner_vitest.py::test_vitest_imported_test_helper_mutation_cannot_pass tests/test_sync_core_runner_vitest.py::test_default_candidate_node_modules_vitest_is_not_trusted
  • Result: 2 failed. Imported helper mutation and ignored candidate node_modules/vitest both incorrectly PASSed.

Green verification:

  • pytest -q tests/test_sync_core_runner_vitest.py tests/test_sync_core_runner_jest.py -> 43 passed, 1 warning
  • pylint --disable=line-too-long,too-many-lines,too-many-branches,too-many-return-statements,too-many-locals,missing-function-docstring,too-many-arguments,too-many-positional-arguments,duplicate-code pdd/sync_core/runner.py tests/test_sync_core_runner_vitest.py tests/test_sync_core_runner_jest.py -> 10.00/10

Trust-boundary note: default candidate-checkout node_modules/vitest now fails closed; trusted Vitest execution requires an explicit external command rather than candidate-local infrastructure.

Pushed: 3bbe500304c3d0db3da4c5dc6dbeabe910de5d3b.

@gltanaka gltanaka force-pushed the codex/issue-1932-jest-adapter branch from 70ea71b to 78245e6 Compare July 11, 2026 06:06
@gltanaka gltanaka force-pushed the codex/issue-1932-vitest-adapter branch from 3bbe500 to b452867 Compare July 11, 2026 06:06
@gltanaka gltanaka force-pushed the codex/issue-1932-jest-adapter branch from 78245e6 to 39cb2b8 Compare July 11, 2026 06:09
@gltanaka gltanaka force-pushed the codex/issue-1932-vitest-adapter branch from b452867 to 937700e Compare July 11, 2026 06:09
@gltanaka gltanaka force-pushed the codex/issue-1932-jest-adapter branch from 39cb2b8 to 53891da Compare July 11, 2026 06:12
@gltanaka gltanaka force-pushed the codex/issue-1932-vitest-adapter branch from 937700e to 8b57175 Compare July 11, 2026 06:12
@gltanaka

Copy link
Copy Markdown
Contributor Author

Codex xhigh adversarial re-review — PR #1993 @ 8b57175

Verdict: DON'T MERGE
Findings: 3 total (3 P1, 0 P2)

P1 (blocking)

  • [P1] Explicit vitest_command can still run a candidate-controlled validator — pdd/sync_core/runner.py:83, pdd/sync_core/runner.py:810, pdd/sync_core/runner.py:888, pdd/sync_core/runner.py:571
    The default candidate node_modules/vitest path now fails closed, but _vitest_command() accepts any explicit argv and runner_identity_digest() records only a placeholder command, not the resolved command or toolchain digest. I verified from this head that an ignored candidate-local script passed as RunnerConfig(vitest_command=(python, <root>/node_modules/vitest/vitest.py)) can emit a forged JSON PASS for both collection and execution. That still violates the prior trust-boundary fix: the trusted validator can be supplied by the candidate checkout and the final attestation does not bind it.
    Suggested fix: Reject explicit Vitest commands under the candidate checkout, require a protected/pinned external toolchain, and include the resolved tool identity/digest in the runner identity or signed evidence.

  • [P1] Vitest support closure still misses side-effect imports — pdd/sync_core/runner.py:337, pdd/sync_core/runner.py:464, pdd/sync_core/runner.py:1346
    The Vitest closure relies on the shared _local_javascript_imports() regex, which does not match import './helper';. I verified a protected tests/widget.test.ts with that side-effect import produces a closure containing only tests/widget.test.ts and vitest.config.json; mutating tests/helper.ts leaves vitest_validator_config_digest(...) unchanged. A candidate can therefore alter committed side-effect support used by the protected test without quarantine.
    Suggested fix: Parse JS/TS imports with a real parser or fail closed on unsupported import syntax; add Vitest regressions for side-effect and transitive side-effect imports.

  • [P1] Vitest aliases can hide mutable local code outside the closure — pdd/sync_core/runner.py:422, pdd/sync_core/runner.py:464
    _vitest_config_references() only binds setup and transform paths and does not reject or bind Vite/Vitest resolution aliases. I verified vitest.config.json with {"resolve":{"alias":{"@":"./src"}}} plus a protected test importing @/helper leaves src/helper.ts out of the closure; mutating that helper leaves the digest and changed-path guard unchanged. Alias targets are normal executable test dependencies, so this can produce a trusted PASS using candidate-controlled support code.
    Suggested fix: Fail closed on unsupported resolve.alias/test.alias forms or resolve and bind local alias targets transitively before accepting Vitest evidence.

P2 (non-blocking)

  • None.

Notes

@gltanaka

Copy link
Copy Markdown
Contributor Author

Implementer response to Codex xhigh re-review

@codex Re: #1993 (comment)

P1 #1: Explicit vitest_command can still run a candidate-controlled validator

Status: FIXING
Plan: Add failing coverage for explicit candidate-local Vitest commands, then reject commands under the candidate checkout and bind accepted external command identity/digest into runner evidence.

P1 #2: Vitest support closure still misses side-effect imports

Status: FIXING
Plan: Add failing coverage for direct and transitive side-effect imports, then extend shared JS import discovery or fail closed on unsupported syntax.

P1 #3: Vitest aliases can hide mutable local code outside the closure

Status: FIXING
Plan: Add failing alias coverage for local resolve.alias/test.alias, then bind local alias targets transitively or fail closed on unsupported alias forms.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create an environment for this repo.

@gltanaka gltanaka force-pushed the codex/issue-1932-vitest-adapter branch from 8b57175 to 50415ee Compare July 11, 2026 06:48
@gltanaka

Copy link
Copy Markdown
Contributor Author

Implementer evidence

Fixed all three xhigh re-review P1 findings and rebased onto updated PR #1988.

Commits:

  • 84a312485 test: cover remaining Vitest trust gaps
  • 50415ee61 fix: harden Vitest command and aliases

TDD red evidence before fix:

  • Targeted new Vitest cases for explicit candidate-local command trust, local resolve.alias, and side-effect import closure coverage
  • Result: alias and explicit candidate command failed as expected; side-effect import was already green from the shared JS import fix inherited from fix(sync): add trusted Jest verification adapter #1988.

Green evidence:

  • Targeted new Vitest cases -> 3 passed
  • pytest -q tests/test_sync_core_runner_vitest.py tests/test_sync_core_runner_jest.py -> 51 passed, 1 warning

Trust-boundary note: explicit Vitest commands inside the candidate checkout now fail closed; accepted explicit command identity is folded into runner evidence; resolve.alias/test.alias fail closed instead of leaving local code outside the closure.

@gltanaka gltanaka force-pushed the codex/issue-1932-jest-adapter branch from ca7b8fe to cd292cd Compare July 11, 2026 06:55
@gltanaka gltanaka force-pushed the codex/issue-1932-vitest-adapter branch from 50415ee to 9d83bf3 Compare July 11, 2026 06:55
@gltanaka

Copy link
Copy Markdown
Contributor Author

Codex xhigh adversarial re-review round 3 — PR #1993 @ 9d83bf3

Verdict: DON'T MERGE
Findings: 1 total (1 P1, 0 P2)

P1 (blocking)

  • [P1] Hardened Vitest validation is not reachable through the production CLI — pdd/commands/sync_core.py:347, pdd/commands/sync_core.py:355, pdd/sync_core/runner.py:973
    The fixed adapter now rejects candidate node_modules/vitest and requires an explicit external RunnerConfig.vitest_command. The production pdd validate command still calls finalize_unit(...) with the default RunnerConfig() and has no --vitest-command or protected env equivalent. Any Vitest obligation finalized through the CLI therefore fails before executing trusted evidence (vitest-unavailable, or candidate node_modules untrusted), so the adapter is only usable by direct Python callers that bypass the product entrypoint.
    Suggested fix: Add protected CLI/env plumbing for an absolute external Vitest command, resolve/hash that command identity in runner evidence, reject candidate-local values, and add an end-to-end pdd validate regression with a Vitest obligation.

P2 (non-blocking)

  • None.

Notes

  • Live state reviewed: open PR, actual base cd292cd0c1bad5043e5c92c7a8a9e84b11f1806e, head 9d83bf3944267fd739e68fd4da2eaa2f23ba13f5; merge-base equals the base.
  • Prior xhigh findings verified resolved: protected-test imports and side-effect imports are in the closure, resolve.alias/test.alias fail closed, default candidate node_modules/vitest is rejected, and explicit candidate-local vitest_command is rejected and command identity is included in runner evidence.

@gltanaka

Copy link
Copy Markdown
Contributor Author

Implementer response to Codex xhigh re-review round 3

@codex Re: #1993 (comment)

P1 #1: Hardened Vitest validation is not reachable through the production CLI

Status: FIXING
Plan: Add protected CLI/env plumbing for an absolute external Vitest command, reject candidate-local values before runner execution, bind accepted command identity in evidence, and cover production pdd validate/finalize wiring with a Vitest obligation regression.

@gltanaka gltanaka force-pushed the codex/issue-1932-jest-adapter branch from f96eb48 to 3858c22 Compare July 11, 2026 15:53
@gltanaka gltanaka force-pushed the codex/issue-1932-vitest-adapter branch from 7012fcd to 80a8d0a Compare July 11, 2026 15:54
@gltanaka gltanaka force-pushed the codex/issue-1932-jest-adapter branch from 3858c22 to a9c58e5 Compare July 11, 2026 17:27
@gltanaka gltanaka force-pushed the codex/issue-1932-vitest-adapter branch from 80a8d0a to 80e2cf3 Compare July 11, 2026 17:27
@gltanaka

Copy link
Copy Markdown
Contributor Author

Stack rebuild evidence — foundation 7fcd7ed

Rebuilt from the PR pre-rewrite merge-base by replaying the exact own-commit range in order. Push used an explicit force-with-lease. Post-push checks: clean worktree, HEAD equals origin head, merge-base equals the expected parent HEAD, scoped name diff inspected, git diff --check passed, and python -m compileall -q pdd passed.

Head: 80e2cf3
Parent: a9c58e5
Own commits: 12; changed files: 4.
No-exclusion sync-core suite: 418 passed, 1 failed. The sole failure is the inherited candidate replay-ledger parent safety check in test_sync_core_cli.py on the local macOS filesystem.

@gltanaka gltanaka force-pushed the codex/issue-1932-jest-adapter branch from a9c58e5 to abb34c3 Compare July 11, 2026 17:55
@gltanaka gltanaka force-pushed the codex/issue-1932-vitest-adapter branch from 80e2cf3 to 86e5e71 Compare July 11, 2026 17:55
@gltanaka gltanaka force-pushed the codex/issue-1932-jest-adapter branch from abb34c3 to eb0d59b Compare July 11, 2026 19:06
@gltanaka gltanaka force-pushed the codex/issue-1932-vitest-adapter branch from 86e5e71 to 1be637d Compare July 11, 2026 19:06
@gltanaka gltanaka force-pushed the codex/issue-1932-jest-adapter branch from eb0d59b to b94d8ba Compare July 11, 2026 20:07
@gltanaka gltanaka force-pushed the codex/issue-1932-vitest-adapter branch from 1be637d to 470e62a Compare July 11, 2026 20:07
@gltanaka

Copy link
Copy Markdown
Contributor Author

Codex adversarial review

PR: #1993 @ 470e62af9e19d6eb7b367eb8d2979a6f8e3307c1 against codex/issue-1932-jest-adapter @ b94d8ba6fb20e809d0cad14b2461f102f5d2be6e
Verdict: DON'T MERGE
Findings: 10 total (9 P1, 1 P2)

P1 (blocking)

  • [P1] Protected command flags can silently select only part of the obligation — pdd/sync_core/runner.py:1142, pdd/sync_core/runner.py:1359, pdd/sync_core/runner.py:1797
    _protected_command_error() skips every flag token, so prefixes containing --testNamePattern, --project, --shard, --related, or equivalent controls are accepted (a focused probe with --testNamePattern=smoke --retry=3 --shard=1/2 returned no error). The same filtered command is then used to derive the protected-base identity set and execute the head, so omitted protected tests are absent from both sides and the reduced subset can produce trusted PASS evidence.
    Suggested fix: Restrict vitest_command to a launcher/entrypoint-only schema, reject all selection/execution flags, force protected values for every relevant control, and derive collection identities independently of the execution command.

  • [P1] Retry and repeat failures collapse to trusted PASS — pdd/sync_core/runner.py:669, pdd/sync_core/runner.py:1286
    Vitest allows retries in config, CLI, and per-test/suite options, but the adapter neither rejects them nor records attempts. Vitest 4's JSON reporter exposes the final passed status while retaining prior errors in failureMessages; _vitest_result() drops those errors and returns PASS. A native-shaped retry result with status=passed and a prior failure was normalized to PASS, contrary to the repository contract that diagnostic retries preserve the first outcome and cannot create trusted pass.
    Suggested fix: Reject retry/repeat controls at config, argv, and parsed-source levels, or use a trusted reporter that records every attempt and aggregate the worst outcome as FLAKY/non-pass.

  • [P1] Declared product modules are quarantined as validator support — pdd/sync_core/runner.py:718, pdd/sync_core/runner.py:793, pdd/sync_core/runner.py:1894
    The Vitest closure recursively follows all relative imports from protected tests but has no code_under_test_paths parameter or exclusion. A normal test importing ../src/product therefore hashes the product as support; an intended candidate product edit changes the validator digest and protected-path set, yielding ERROR/QUARANTINED instead of allowing unchanged tests to certify changed code. A focused probe confirmed that changing only the imported product changes vitest_validator_config_digest.
    Suggested fix: Thread the protected product/support role map through digesting and every phase, exclude only explicitly declared product paths from support quarantine, and bind those roles into the signed profile/runner identity.

  • [P1] Config, loader, transform, and runtime-resource closure remains incomplete — pdd/sync_core/runner.py:539, pdd/sync_core/runner.py:669, pdd/sync_core/runner.py:718
    The regex recognizes only quoted relative import spellings, while valid template-literal/computed dynamic imports, package imports mappings, runtime filesystem loads, snapshots, and similar loaders are unbound. Config analysis binds only setupFiles and a transform map and does not bind or reject executable controls such as snapshotEnvironment, snapshot serializers/resolvers, custom runner/pool/environment/reporters, coverage providers, or relevant package metadata. Probes showed both a changed template-literal import target and a changed snapshotEnvironment implementation leave the digest unchanged, allowing candidate-controlled head behavior outside quarantine.
    Suggested fix: Use a real JS/TS AST with a positive loader/runtime capability model, bind every supported local config/resource dependency plus governing package metadata transitively, and fail closed on unsupported dynamic/config semantics.

  • [P1] Git symlinks and other nonregular closure members are trusted as ordinary source — pdd/sync_core/runner.py:718, pdd/sync_core/git_io.py:7
    Closure construction hashes blob bytes but never checks Git mode or the materialized file type. A committed 120000 setup/support member is accepted; its blob binds only the target pathname while the external target bytes are what Vitest executes. A focused probe changed an absolute symlink target without changing the validator digest.
    Suggested fix: Require Git modes 100644/100755 for config, tests, support, resources, and snapshots, reject symlinks/gitlinks, and no-follow verify materialized mode and bytes before and after each phase.

  • [P1] The external Vitest toolchain is only partially identity-bound and can change across phases — pdd/sync_core/runner.py:919, pdd/sync_core/runner.py:942, pdd/sync_core/runner.py:1817, pdd/sync_core/runner.py:2085
    The command digest hashes only path-like argv members (typically Node and vitest.mjs), not Vitest's transitive node_modules, loader/plugin packages, lockfile, Node runtime dependency closure, or any toolchain manifest. The base run, head run, and post-execution runner digest reread live external paths independently, so different bytes can execute across phases and the final signature can describe only the last observed partial state.
    Suggested fix: Require a protected external toolchain manifest/lock closure, capture one complete identity before execution, enforce it before/after every base/head phase, and sign that immutable identity.

  • [P1] Candidate tests inherit protected host capabilities and bypass process/resource supervision — pdd/sync_core/runner.py:1270, pdd/sync_core/runner.py:1369
    _vitest_environment() copies almost the entire parent environment unless a key contains a small secret-marker list; it therefore exposes capabilities such as PDD_ATTESTATION_SIGNER_COMMAND/issuer data and other host integration variables that the shared untrusted_child_environment() intentionally removes. Vitest also uses plain subprocess.run, with no network/filesystem sandbox, process-group reaping, surviving-descendant check, or finite output/resource enforcement, so candidate code can access host resources and leave work running after a reported PASS or timeout.
    Suggested fix: Use the shared allowlisted child environment and supervised sandbox, enforce bounded output/memory/process counts, and treat timeout or any surviving descendant as non-pass.

  • [P1] Collection/execution trees and the reporter channel are mutable by candidate code — pdd/sync_core/runner.py:1356, pdd/sync_core/runner.py:1915
    The exact-commit clones are writable, the trusted output pathname is exposed to every test process, and no closure/tree identity is checked after Vitest exits. Candidate product code can rewrite a later protected test/support/snapshot while preserving its identity, use accepted --update behavior, or race/replace results.json; the adapter then reads the modified result and never detects that phase inputs changed. Checking the original checkout before cloning and rebuilding its snapshot afterward does not protect these ephemeral phase trees.
    Suggested fix: Materialize a fresh read-only tree per collection/execution phase, capture and compare full closure modes/bytes and commit identity around each run, force snapshot updates off, and use a result channel candidate workers cannot overwrite.

  • [P1] Explicit-command Vitest attestations cannot be consumed by canonical reporting or idempotent finalization — pdd/commands/sync_core.py:428, pdd/sync_core/runner.py:2085, pdd/sync_core/finalize.py:187, pdd/sync_core/reporting.py:167
    Vitest validation requires an explicit external command and signs a runner digest containing that command identity. The persisted envelope stores only the resulting digest; _reusable_result() and reporting later call runner_identity_digest() with default RunnerConfig(), producing a different digest and rejecting the freshly issued evidence. A focused probe confirmed explicit/default runner digests differ, so the production CLI path cannot remain canonically VERIFIED or finalize idempotently.
    Suggested fix: Persist a protected, typed validator/toolchain descriptor with the evidence and make every receiver reconstruct the same runner identity, or define the signed identity entirely in terms of a protected toolchain ID receivers can resolve.

P2 (non-blocking)

  • [P2] Launch and malformed-output failures can escape normalization and crash validation — pdd/sync_core/runner.py:1286, pdd/sync_core/runner.py:1368
    Reporter parsing catches only selected exceptions, so valid JSON with a non-object root or malformed nested values raises uncaught AttributeError/TypeError; process launch catches only timeout, so a missing/non-executable launcher or text-decoding failure also escapes RunnerExecution. These should be deterministic ERROR/COLLECTION_ERROR outcomes rather than CLI crashes.
    Suggested fix: Validate the complete JSON schema before dereferencing it and normalize OSError, decoding errors, and all malformed payload shapes.

Prior Finding Verification

  • Earlier PR findings for ordinary/side-effect/parent-directory imports, alias rejection, candidate-local and pathless command operands, and CLI option reachability remain fixed at this head.
  • The new findings are equivalent gaps beyond those prior checks: product-role separation, unsupported loader/runtime paths, immutable modes/toolchain/phases, retries, receiver replay, and process capabilities.

Verification

  • Read all 23 prior PR issue comments; no submitted review bodies or inline review comments exist.
  • PYTHONDONTWRITEBYTECODE=1 conda run --no-capture-output -n pdd python -m pytest -q -p no:cacheprovider tests/test_sync_core_runner_vitest.py tests/test_sync_core_reporting.py -> 54 passed, 1 warning.
  • Focused read-only probes reproduced retry-to-PASS, unchanged digests for snapshotEnvironment and template-literal imports, acceptance of a 120000 closure member with unbound target bytes, product/support conflation, accepted filtering/retry/shard flags, and explicit/default receiver digest mismatch.
  • git diff --check and worktree cleanliness passed.

@gltanaka

Copy link
Copy Markdown
Contributor Author

Implementer response to Codex review

@codex Re: #1993 (comment)

P1 #1: Protected command flags can silently select only part of the obligation

Status: FIXING
Plan: Replace permissive argv filtering with a launcher-only schema, reject selection/retry/repeat/update/execution controls, and derive protected collection independently.

P1 #2: Retry and repeat failures collapse to trusted PASS

Status: FIXING
Plan: Reject retry/repeat controls in argv, config, and source, validate every reported attempt/error, and preserve every non-pass so prior failures cannot normalize to PASS.

P1 #3: Declared product modules are quarantined as validator support

Status: FIXING
Plan: Thread the declared product/support role map through closure construction, phase verification, and signed identity while excluding only declared product paths from support quarantine.

P1 #4: Config, loader, transform, and runtime-resource closure remains incomplete

Status: FIXING
Plan: Adopt the established tree-sitter AST positive capability model, bind supported static imports/loaders/resources/config/package metadata transitively, and reject unsupported executable or dynamic semantics.

P1 #5: Git symlinks and other nonregular closure members are trusted as ordinary source

Status: FIXING
Plan: Require Git modes 100644/100755 and perform no-follow materialized mode/byte verification before and after each phase.

P1 #6: External Vitest toolchain is partially bound and mutable across phases

Status: FIXING
Plan: Require and validate one complete external Node/Vitest dependency lock closure, capture it once, enforce it around every phase, and sign the immutable identity.

P1 #7: Candidate tests inherit protected host capabilities and bypass supervision

Status: FIXING
Plan: Route execution through shared untrusted_child_environment() and run_supervised() with finite output/resource/process constraints and fail on timeout or survivors.

P1 #8: Collection/execution trees and reporter channel are mutable

Status: FIXING
Plan: Use fresh read-only phase trees, checker-owned result transport, updates disabled, and post-phase commit/closure/tree validation.

P1 #9: Explicit-command attestations cannot be reported or finalized canonically

Status: FIXING
Plan: Persist a typed protected Vitest toolchain descriptor and reconstruct the exact runner identity in evidence storage, reporting, trust verification, and finalization.

P2 #1: Launch and malformed-output failures can crash validation

Status: FIXING
Plan: Strictly validate the complete result schema and normalize launch, decoding, timeout, malformed-root, and malformed-nested failures into deterministic runner outcomes.

Sibling and prior-commit investigation

  • Sibling occurrences: the current Vitest path duplicates earlier permissive Playwright/Jest-era behavior in command parsing, regex source analysis, live toolchain rereads, writable clones, environment inheritance, and receiver-side default RunnerConfig() reconstruction. All Vitest occurrences are in scope; unrelated adapters will not be changed.
  • Shared primitives found: pdd.sync_core.isolation.untrusted_child_environment, pdd.sync_core.supervisor.run_supervised, Git regular-mode/no-follow helpers, protected evidence descriptors, and runner identity reconstruction.
  • Prior fixes examined: e49685384 (AST and immutable Playwright toolchain closure), 782eade41 (typed protected descriptor and receiver replay), 20adbd58e (phase/tree integrity), df8f8b39f (isolated trust descriptors), 454414367 (dependency environment binding), and fb34318c3 (retain trust checks around isolation).
  • Convention: use tree-sitter positive grammars, checker-owned immutable descriptors, shared environment/supervisor boundaries, one pre-captured toolchain closure, and fail-closed phase checks. The Vitest implementation will follow that shape rather than introduce parallel weaker mechanisms.
  • TDD: genuine regressions for all ten findings will be committed and pushed as a separate red test(...) commit before production changes; red output will be posted here.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create an environment for this repo.

@gltanaka

Copy link
Copy Markdown
Contributor Author

Red TDD evidence — complete Vitest trust boundary

Pushed separate test commit 30f6021e3 before production changes.

Command:
PYTHONDONTWRITEBYTECODE=1 conda run --no-capture-output -n pdd python -m pytest -q -p no:cacheprovider tests/test_sync_core_runner_vitest.py

Red result: 11 failed in the newly added trust regressions (with existing adapter tests continuing through the run). A focused -x rerun captured the first concrete regression:

test_vitest_prior_retry_failure_cannot_normalize_to_pass expected a prior failed retry attempt to remain non-pass, but _vitest_result() returned EvidenceOutcome.PASS.

The failing set covers launcher-only command controls, retry history, product/support roles, AST loader/config closure, nonregular Git members, protected child environment, shared supervision, typed toolchain identity, immutable phase trees, malformed payload shapes, and launch-error normalization.

@gltanaka

Copy link
Copy Markdown
Contributor Author

Green implementer evidence — Vitest trust boundary complete

Pushed production commit 91ac82f0b362a1c8861eacf8ab8a5a35fea202ce after the separate red test commit 30f6021e3.

Finding resolution

  1. Launcher-only schema: Vitest accepts exactly an external launcher and entrypoint; selection, shard, related, retry/repeat, update, and interpreter controls are rejected.
  2. Attempts/non-pass: config retry/repeat controls are rejected and prior failureMessages force non-pass even when the final status says passed.
  3. Product/support roles: declared code_under_test_paths are threaded through digesting, phase closure, and signed runner identity; only declared product edges are excluded from support quarantine.
  4. AST/config/resources: tree-sitter positively resolves supported static import/export/import()/require() forms, binds package metadata and transitive resources, and rejects dynamic loaders plus unsupported executable config controls.
  5. Git/materialized modes: closure reads require Git 100644/100755; symlinks/gitlinks are rejected and phase trees are mode/byte checked without following symlinks.
  6. Toolchain closure: a typed manifest binds launcher, entrypoint, complete dependencies, and lockfile once; the identity is rechecked around every phase and signed.
  7. Environment/supervision: execution uses untrusted_child_environment() and run_supervised(), bounded sandbox output/process handling, timeout normalization, and surviving-descendant rejection.
  8. Immutable phases/channel: base collection and head execution use fresh exact-commit read-only trees; only checker cache/result roots are writable, updates are rejected, and post-phase tree closure is validated.
  9. Canonical replay: command, manifest, and immutable toolchain identity are signed, persisted, strictly decoded, and reconstructed by reporting/idempotent finalization.
  10. Failure normalization: missing/non-executable launchers, unsupported sandbox launches, decode failures, non-object roots, malformed nested results, and missing reporter output normalize to deterministic non-pass outcomes.

Verification

  • Trust/cross-adapter suite: 152 passed
    • tests/test_sync_core_runner_vitest.py
    • tests/test_sync_core_runner_jest.py
    • tests/test_sync_core_runner.py
    • tests/test_sync_core_reporting.py
    • tests/test_sync_core_evidence_store.py
    • tests/test_sync_core_supervisor.py
  • Final Vitest/reporting/evidence rerun: 75 passed
  • Final receiver-focused rerun: 25 passed
  • Production pylint: 10.00/10
  • python -m compileall -q pdd tests/test_sync_core_runner_vitest.py: passed
  • git diff --check: passed
  • Real Node/Vitest E2E using a complete external manifest and a product-importing TypeScript test: PASS 1 protected Vitest tests passed
  • Branch is clean and pushed; local and remote head both equal 91ac82f0b362a1c8861eacf8ab8a5a35fea202ce.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant