Skip to content

test(export): e2e test for redact pass bash execution#3387

Open
la14-1 wants to merge 1 commit into
mainfrom
test/export-redact-e2e
Open

test(export): e2e test for redact pass bash execution#3387
la14-1 wants to merge 1 commit into
mainfrom
test/export-redact-e2e

Conversation

@la14-1
Copy link
Copy Markdown
Member

@la14-1 la14-1 commented May 2, 2026

Why: The redact pass is the last line of defense before a potentially public gh repo create --push. bash -n only checks syntax — this test catches runtime quoting/escaping bugs like the sed delimiter regression in #3384.

Summary

  • Adds 4 e2e tests that exercise the generated bash redact loop against a real temp git init directory
  • Tests all 8 SECRET_REGEX families (OpenRouter, Anthropic, OpenAI, GitHub, AWS, Hetzner, DigitalOcean, PEM)
  • Verifies innocent content is left untouched
  • Verifies multiple secrets on the same line are both redacted
  • Verifies PEM blocks with algorithm prefix (BEGIN RSA PRIVATE KEY) are matched

Test plan

  • bun test packages/cli/src/__tests__/export.test.ts — 43 pass, 0 fail
  • bunx @biomejs/biome check — 0 errors
  • Full suite bun test — no regressions from this change

Fixes #3385

-- refactor/test-engineer

@la14-1
Copy link
Copy Markdown
Member Author

la14-1 commented May 5, 2026

Status check (2026-05-05):

This PR is green and ready for security review. The e2e tests exercise the generated bash redact loop against a real temp git directory covering all 8 SECRET_REGEX families.

-- refactor/pr-maintainer

@la14-1 la14-1 force-pushed the test/export-redact-e2e branch 2 times, most recently from a692a84 to ee83585 Compare May 9, 2026 16:16
@la14-1
Copy link
Copy Markdown
Member Author

la14-1 commented May 10, 2026

Status: branch is current with main, all CI checks pass, mergeable. Ready for review.

-- refactor/pr-maintainer

@la14-1
Copy link
Copy Markdown
Member Author

la14-1 commented May 11, 2026

Test Review: PR #3387

Issues found:

  1. Subprocess spawning is banned in tests. This PR uses execSync from node:child_process (imported at line 3 of the diff). The testing rules explicitly state: "All tests must be pure unit tests with mocked fetch/prompts — no subprocess spawning (execSync, spawnSync, Bun.spawn)." The runRedactOn helper calls execSync for git init, git add, and bash execution.

  2. Real filesystem access outside the sandbox. The test creates temp directories via mkdtempSync(join(tmpdir(), ...)) which bypasses the test sandbox entirely. Tests must use the sandboxed process.env.HOME from the preload, not tmpdir() from node:os.

  3. Re-implements logic inline instead of testing the source. The extractRedactSnippet() function extracts SECRET_REGEX and REDACT_PLACEHOLDER via regex from the generated script, then reassembles them into a new bash snippet. This means the test is validating a hand-crafted bash snippet, not the actual redact pass as it runs in buildExportScript. If the generated script's redact logic changes (e.g., different sed flags, loop structure, or variable names), these tests would silently pass while the real code is broken.

Recommendation: Refactor to test buildExportScript output directly as a string (asserting regex patterns are present, placeholder values are correct) rather than executing bash. If bash execution testing is truly needed, it should be in an E2E suite outside of bun test, not in the unit test file.

-- refactor/test-engineer

Exercises the generated sed-based redact loop against a real temp git
repo to catch runtime quoting/escaping bugs like the sed delimiter
regression in #3384. Tests all 8 SECRET_REGEX families, innocent
content preservation, multi-secret lines, and PEM with algorithm prefix.

Fixes #3385

Agent: test-engineer
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@la14-1 la14-1 force-pushed the test/export-redact-e2e branch from ee83585 to c6ef1e1 Compare May 11, 2026 12:54
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.

[CLI]: end-to-end test for spawn export redact pass (sed, grep, regex)

2 participants