Skip to content

test: add standalone reproduction for PowerShell literal classification - #830

Draft
aman84ad wants to merge 1 commit into
wonderwhy-er:mainfrom
aman84ad:diagnostic/issue-731-literal-repro
Draft

aman84ad wants to merge 1 commit into
wonderwhy-er:mainfrom
aman84ad:diagnostic/issue-731-literal-repro

Conversation

@aman84ad

@aman84ad aman84ad commented Oct 9, 2026 •

Copy link
Copy Markdown

The command-name extractor can classify non-executable content in ordinary PowerShell single-quoted strings as a blocked command. This is relevant to the quoting problem discussed in #731 and makes benign script text difficult to distinguish from executable controls during review.

This contribution adds a standalone diagnostic under diagnostics/issue-731. It pins a byte-identical public upstream command-manager.ts snapshot and its MIT license, a closed set of synthetic fixture strings, and a dependency-free Node.js 24 launcher. There are no changes to production source, configuration, approval rules, package scripts, or connector behavior.

The diagnostic records 16 known observations:

  • Six literal-text false positives.
  • Six actual blocked-command controls denied by the unchanged parser.
  • Two ordinary literal cases classified correctly.
  • Two inherited invocation gaps, explicitly reported as false accepts.

An exit status of zero means the pinned behavior was reproduced. It does not mean the parser is fixed, that filtering is complete, or that Windows execution is approved. Ten launcher regression tests check integrity, fixed inputs, complete/known observations, and refusal when a blocked-control result changes.

Verification recorded on Linux with Node.js 24.19.0:

  • node verify.mjs: BUG_REPRODUCED, EXPECTED_OBSERVATIONS_CONFIRMED.
  • node --test --test-isolation=none test/launcher.test.mjs: 10 passed, 0 failed.
  • A fresh copied kit reproduces the same result without workspace dependencies.

Fixture text is never executed. Native PowerShell AST/execution, a full TypeScript/package build, and installed-connector integration remain unrun. This diagnostic does not contain the previously proposed replacement parser or any private rejected request.

Related discussion:
#731
Previously published, unaccepted proposal for maintainer review:
#731 (comment)

Please confirm whether this location and Node 24 prerequisite fit the project, and identify the supported correction and acceptance process. Maintainer review is needed before any change to command validation.

Summary by CodeRabbit

  • Documentation
    • Added an offline diagnostic guide for reproducing and reviewing command-quoting behavior, including expected results and verification limits.
  • Tests
    • Added checks that confirm diagnostic results and reject altered or unexpected inputs.
    • Recorded 16 cases: six false positives, six correctly denied blocked commands, two ordinary command matches, and two existing false accepts.
    • The report notes that native PowerShell, full builds, and installed-connector checks were not performed.

Pin unchanged upstream parser and public synthetic fixtures for issue wonderwhy-er#731.
Record known false positives and false accepts separately from denied controls.
No production parser, policy, connector, or execution changes.
@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 44779213-e9b1-4d8c-9836-93f8cbf09c64

📥 Commits

Reviewing files that changed from the base of the PR and between ea3ed35 and b900902.


📒 Files selected for processing (11)
  • diagnostics/issue-731/README.md
  • diagnostics/issue-731/SHA256SUMS
  • diagnostics/issue-731/fixtures.json
  • diagnostics/issue-731/manifest.json
  • diagnostics/issue-731/node-warning.txt
  • diagnostics/issue-731/observed-results.json
  • diagnostics/issue-731/test/launcher.test.mjs
  • diagnostics/issue-731/upstream/LICENSE
  • diagnostics/issue-731/upstream/command-manager.ts
  • diagnostics/issue-731/verification.json
  • diagnostics/issue-731/verify.mjs

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.



📝 Walkthrough

Walkthrough

Adds an offline Node.js 24 diagnostic kit for a pinned parser snapshot. It defines 16 fixture cases, verifies source and fixture hashes, evaluates the parser in a restricted VM, and records observed results and execution limits.

Changes

Parser diagnostic reproduction

Layer / File(s) Summary
Pinned parser and diagnostic cases
diagnostics/issue-731/manifest.json, diagnostics/issue-731/fixtures.json, diagnostics/issue-731/upstream/command-manager.ts, diagnostics/issue-731/upstream/LICENSE, diagnostics/issue-731/SHA256SUMS
The manifest pins the parser snapshot and fixture hashes. The fixture set records expected and baseline outcomes for 16 cases. The snapshot and its license are included with checksums.
Isolated verifier and launcher checks
diagnostics/issue-731/verify.mjs, diagnostics/issue-731/test/launcher.test.mjs
The verifier checks pinned inputs, evaluates the parser in a restricted VM, validates fixture observations, and emits a JSON report or refusal. Tests cover report contents, fresh-copy verification, and rejection of altered or incomplete inputs.
Diagnostic records and reproduction instructions
diagnostics/issue-731/observed-results.json, diagnostics/issue-731/verification.json, diagnostics/issue-731/README.md, diagnostics/issue-731/node-warning.txt
The records report the 16 case outcomes, runtime and isolation details, test results, and checks not run. The README documents invocation and reproduction limits. The warning file records the Node.js experimental warning.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Other

Sequence Diagram(s)

sequenceDiagram
  participant NodeCLI
  participant verifyDiagnostic
  participant manifest.json
  participant fixtures.json
  participant VM
  participant commandManager
  NodeCLI->>verifyDiagnostic: Run diagnostic
  verifyDiagnostic->>manifest.json: Check metadata and pinned hashes
  verifyDiagnostic->>fixtures.json: Load fixture definitions
  verifyDiagnostic->>VM: Evaluate prepared parser with fixtures
  VM->>commandManager: Extract and validate commands for each fixture
  commandManager-->>VM: Return command names and denial results
  VM-->>verifyDiagnostic: Return observations and isolation state
  verifyDiagnostic-->>NodeCLI: Return JSON report or refusal
Loading

Merge Risk: ⚪ Minimal · up to b9009

This change adds an isolated, offline diagnostic kit for reproducing the parser behavior discussed in issue 731. It does not change runtime behavior of the product, so merge risk is minimal.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. (8 skipped: 8… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the standalone diagnostic added for PowerShell literal classification, which is the main change.
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.

Full details: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. (8 skipped: 8 unsupported.)



  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR


  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

aman84ad commented Oct 9, 2026

Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

This branch has not been deployed

No deployments
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