test: add TOCTOU error path coverage for ssl-bump.ts by Mossaka · Pull Request #1161 · github/gh-aw-firewall

Mossaka · 2026-03-05T19:56:47Z

Summary

Add tests for EEXIST error handling in initSslDb (silently caught when files already exist via 'wx' flag)
Add tests for non-EEXIST errors in initSslDb (EACCES on index.txt and size file - should re-throw)
Add test for non-Error throw handling in generateSessionCa (string rejection path)

These tests cover the error branches introduced by the TOCTOU fix in #1159, improving ssl-bump.ts branch coverage.

Fixes #837

Test plan

All 835 tests pass (npm test)
Build succeeds (npm run build)
Lint passes with 0 errors (npm run lint)
New tests specifically cover the EEXIST/non-EEXIST error paths and string rejection handling

🤖 Generated with Claude Code

Add tests for: - EEXIST error handling (silently caught when files already exist) - Non-EEXIST errors re-thrown (EACCES on index.txt and size file) - Non-Error throw handling in generateSessionCa (string rejection) Fixes #837 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

github-actions · 2026-03-05T19:57:36Z

✅ Coverage Check Passed

Overall Coverage

Metric	Base	PR	Delta
Lines	82.27%	82.41%	📈 +0.14%
Statements	82.17%	82.40%	📈 +0.23%
Functions	82.60%	82.60%	➡️ +0.00%
Branches	74.12%	74.47%	📈 +0.35%

📁 Per-file Coverage Changes (2 files)

File	Lines (Before → After)	Statements (Before → After)
`src/ssl-bump.ts`	90.5% → 90.5% (+0.00%)	88.8% → 90.7% (+1.87%)
`src/docker-manager.ts`	83.0% → 83.6% (+0.55%)	82.4% → 82.9% (+0.53%)

Coverage comparison generated by scripts/ci/compare-coverage.ts

Copilot

Pull request overview

Adds targeted tests to improve branch coverage for TOCTOU-related error handling paths in ssl-bump.ts, focusing on initSslDb filesystem errors and generateSessionCa non-Error rejections.

Changes:

Added coverage for generateSessionCa handling of non-Error (string) rejections.
Added coverage for initSslDb behavior on EEXIST (should be swallowed) vs non-EEXIST errors (should re-throw).
Added permission-based tests to simulate EACCES during DB initialization.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-05T19:58:26Z

src/ssl-bump.test.ts

+      // Create ssl_db directory, then make it read-only so writeFileSync fails with EACCES
+      const sslDbPath = path.join(tempDir, 'ssl_db');
+      fs.mkdirSync(path.join(sslDbPath, 'certs'), { recursive: true });
+      // Make ssl_db read-only so index.txt creation fails with EACCES
+      fs.chmodSync(sslDbPath, 0o555);
+
+      try {
+        await expect(initSslDb(tempDir)).rejects.toThrow(/EACCES/);
+      } finally {
+        // Restore permissions for cleanup
+        fs.chmodSync(sslDbPath, 0o700);
+      }
+    });
+
+    it('should re-throw non-EEXIST errors from size file writeFileSync', async () => {
+      // Create full structure with index.txt, then make dir read-only
+      // so only the size file creation fails
+      const sslDbPath = path.join(tempDir, 'ssl_db');
+      fs.mkdirSync(path.join(sslDbPath, 'certs'), { recursive: true });
+      // Pre-create index.txt so it hits EEXIST (silently caught), then
+      // size file creation will fail because dir is read-only
+      fs.writeFileSync(path.join(sslDbPath, 'index.txt'), '');
+      fs.chmodSync(sslDbPath, 0o555);
+
+      try {
+        await expect(initSslDb(tempDir)).rejects.toThrow(/EACCES/);
+      } finally {
+        fs.chmodSync(sslDbPath, 0o700);


These tests rely on chmodSync causing EACCES, which is not portable (notably unreliable on Windows, and can behave differently under elevated/CI permissions). This can make the suite fail or become flaky across environments. Prefer mocking fs.writeFileSync to throw an error with code: 'EACCES' for the specific call(s), or conditionally skip these tests on platforms where POSIX permissions don’t apply.

Suggested change

// Create ssl_db directory, then make it read-only so writeFileSync fails with EACCES

const sslDbPath = path.join(tempDir, 'ssl_db');

fs.mkdirSync(path.join(sslDbPath, 'certs'), { recursive: true });

// Make ssl_db read-only so index.txt creation fails with EACCES

fs.chmodSync(sslDbPath, 0o555);

try {

await expect(initSslDb(tempDir)).rejects.toThrow(/EACCES/);

} finally {

// Restore permissions for cleanup

fs.chmodSync(sslDbPath, 0o700);

}

});

it('should re-throw non-EEXIST errors from size file writeFileSync', async () => {

// Create full structure with index.txt, then make dir read-only

// so only the size file creation fails

const sslDbPath = path.join(tempDir, 'ssl_db');

fs.mkdirSync(path.join(sslDbPath, 'certs'), { recursive: true });

// Pre-create index.txt so it hits EEXIST (silently caught), then

// size file creation will fail because dir is read-only

fs.writeFileSync(path.join(sslDbPath, 'index.txt'), '');

fs.chmodSync(sslDbPath, 0o555);

try {

await expect(initSslDb(tempDir)).rejects.toThrow(/EACCES/);

} finally {

fs.chmodSync(sslDbPath, 0o700);

// Create ssl_db directory

const sslDbPath = path.join(tempDir, 'ssl_db');

fs.mkdirSync(path.join(sslDbPath, 'certs'), { recursive: true });

// Mock writeFileSync so that creating index.txt fails with EACCES

const originalWriteFileSync = fs.writeFileSync;

const writeSpy = jest

.spyOn(fs, 'writeFileSync')

.mockImplementation((file: any, data: any, options?: any): any => {

if (typeof file === 'string' && file === path.join(sslDbPath, 'index.txt')) {

const err: any = new Error('EACCES');

err.code = 'EACCES';

throw err;

}

return (originalWriteFileSync as any)(file, data, options);

});

try {

await expect(initSslDb(tempDir)).rejects.toThrow(/EACCES/);

} finally {

writeSpy.mockRestore();

}

});

it('should re-throw non-EEXIST errors from size file writeFileSync', async () => {

// Create full structure with index.txt so it hits EEXIST (silently caught),

// then simulate a failure only for the size file creation

const sslDbPath = path.join(tempDir, 'ssl_db');

fs.mkdirSync(path.join(sslDbPath, 'certs'), { recursive: true });

// Pre-create index.txt

fs.writeFileSync(path.join(sslDbPath, 'index.txt'), '');

// Mock writeFileSync so that creating the size file fails with EACCES

const originalWriteFileSync = fs.writeFileSync;

const writeSpy = jest

.spyOn(fs, 'writeFileSync')

.mockImplementation((file: any, data: any, options?: any): any => {

if (typeof file === 'string' && file === path.join(sslDbPath, 'size')) {

const err: any = new Error('EACCES');

err.code = 'EACCES';

throw err;

}

return (originalWriteFileSync as any)(file, data, options);

});

try {

await expect(initSslDb(tempDir)).rejects.toThrow(/EACCES/);

} finally {

writeSpy.mockRestore();

Copilot · 2026-03-05T19:58:26Z

src/ssl-bump.test.ts

+      fs.chmodSync(sslDbPath, 0o555);
+
+      try {
+        await expect(initSslDb(tempDir)).rejects.toThrow(/EACCES/);


Asserting via toThrow(/EACCES/) depends on the error message text, which can vary by OS/runtime. A more stable assertion is to check the error shape (e.g., code: 'EACCES') using await expect(...).rejects.toMatchObject({ code: 'EACCES' }) (or whatever the implementation throws), which reduces cross-platform brittleness.

Copilot · 2026-03-05T19:58:26Z

src/ssl-bump.test.ts

+      fs.chmodSync(sslDbPath, 0o555);
+
+      try {
+        await expect(initSslDb(tempDir)).rejects.toThrow(/EACCES/);


Asserting via toThrow(/EACCES/) depends on the error message text, which can vary by OS/runtime. A more stable assertion is to check the error shape (e.g., code: 'EACCES') using await expect(...).rejects.toMatchObject({ code: 'EACCES' }) (or whatever the implementation throws), which reduces cross-platform brittleness.

Copilot · 2026-03-05T19:58:27Z

src/ssl-bump.test.ts

+      });
+
+      await expect(generateSessionCa({ workDir: tempDir })).rejects.toThrow(
+        'Failed to generate SSL Bump CA: unexpected string rejection'


The assertion matches the full error string exactly, which is brittle if the implementation changes wording/punctuation while still preserving the intended behavior. Consider asserting on key substrings (e.g., regex containing both Failed to generate SSL Bump CA and unexpected string rejection) to keep the test focused on behavior rather than exact formatting.

Suggested change

'Failed to generate SSL Bump CA: unexpected string rejection'

/Failed to generate SSL Bump CA[\s\S]*unexpected string rejection/

Copilot · 2026-03-05T19:58:27Z

src/ssl-bump.test.ts

+      expect(fs.readFileSync(path.join(sslDbPath, 'size'), 'utf-8')).toBe('42\n');
+    });
+
+    it('should re-throw non-EEXIST errors from writeFileSync', async () => {


The two non-EEXIST tests duplicate setup/teardown patterns (create structure → chmod → expect reject → restore). Consider extracting a small helper (e.g., withReadOnlyDir(sslDbPath, fn)) to reduce repetition and make the intent of each case (which write fails) stand out.

Copilot · 2026-03-05T19:58:27Z

src/ssl-bump.test.ts

+      }
+    });
+
+    it('should re-throw non-EEXIST errors from size file writeFileSync', async () => {


The two non-EEXIST tests duplicate setup/teardown patterns (create structure → chmod → expect reject → restore). Consider extracting a small helper (e.g., withReadOnlyDir(sslDbPath, fn)) to reduce repetition and make the intent of each case (which write fails) stand out.

github-actions · 2026-03-05T20:01:06Z

🧪 Build Test: Bun

Project	Install	Tests	Status
elysia	✅	1/1	PASS
hono	✅	1/1	PASS

Overall: ✅ PASS

Bun v1.3.10

Generated by Build Test Bun for issue #1161

github-actions · 2026-03-05T20:01:08Z

Smoke test results:
GitHub MCP merged PRs: ✅ fix(security): eliminate TOCTOU race conditions in ssl-bump.ts, fix(security): stop logging partial token values
Safeinputs gh PR list: ✅ test: add TOCTOU error path coverage for ssl-bump.ts, fix(squid): block direct IP connections that bypass domain filtering
Playwright title check: ✅
Tavily search: ❌ (tool unavailable)
File write + cat: ✅
Build npm ci && npm run build: ✅
Overall: FAIL

🔮 The oracle has spoken through Smoke Codex for issue #1161

github-actions · 2026-03-05T20:01:09Z

Go Build Test Results

Project	Download	Tests	Status
color	✅	PASS	✅ PASS
env	✅	PASS	✅ PASS
uuid	✅	PASS	✅ PASS

Overall: ✅ PASS

Generated by Build Test Go for issue #1161

github-actions · 2026-03-05T20:01:25Z

Smoke Test Results

✅ GitHub MCP: #1159 fix(security): eliminate TOCTOU race conditions in ssl-bump.ts | #1158 fix(security): stop logging partial token values
✅ Playwright: github.com title contains "GitHub"
✅ File Write: /tmp/gh-aw/agent/smoke-test-claude-22734171787.txt created
✅ Bash: File verified via cat

Overall: PASS

💥 [THE END] — Illustrated by Smoke Claude for issue #1161

github-actions · 2026-03-05T20:01:27Z

.NET Build Test Results

Project	Restore	Build	Run	Status
hello-world	✅	✅	✅	PASS
json-parse	✅	✅	✅	PASS

Overall: PASS ✅

Run output

hello-world:

Hello, World!
```

**json-parse:**
```
{
  "Name": "AWF Test",
  "Version": 1,
  "Success": true
}
Name: AWF Test, Success: True

Generated by Build Test .NET for issue #1161

github-actions · 2026-03-05T20:01:27Z

Build Test: Deno ✅

Project	Tests	Status
oak	1/1	✅ PASS
std	1/1	✅ PASS

Overall: PASS

Generated by Build Test Deno for issue #1161

github-actions · 2026-03-05T20:01:32Z

C++ Build Test Results

Project	CMake	Build	Status
fmt	✅	✅	PASS
json	✅	✅	PASS

Overall: PASS ✅

Generated by Build Test C++ for issue #1161

github-actions · 2026-03-05T20:01:46Z

🦀 Rust Build Test Results

Project	Build	Tests	Status
fd	✅	1/1	PASS
zoxide	✅	1/1	PASS

Overall: ✅ PASS

Generated by Build Test Rust for issue #1161

github-actions · 2026-03-05T20:01:53Z

Smoke test results for run 22734171796 — @Mossaka

Test	Result
GitHub MCP (last 2 merged PRs: #1159, #1158)	✅
Playwright (github.com title contains "GitHub")	✅
File write `/tmp/gh-aw/agent/smoke-test-copilot-22734171796.txt`	✅
Bash `cat` verification	✅

Overall: PASS

📰 BREAKING: Report filed by Smoke Copilot for issue #1161

github-actions · 2026-03-05T20:03:08Z

Node.js Build Test Results ✅

Project	Install	Tests	Status
clsx	✅	PASS	PASS
execa	✅	PASS	PASS
p-limit	✅	PASS	PASS

Overall: PASS

Generated by Build Test Node.js for issue #1161

github-actions · 2026-03-05T20:04:43Z

Java Build Test Results

Project	Compile	Tests	Status
gson	✅	1/1	PASS
caffeine	✅	1/1	PASS

Overall: PASS ✅

Generated by Build Test Java for issue #1161

github-actions · 2026-03-05T20:06:31Z

Chroot Version Comparison Results

Runtime	Host Version	Chroot Version	Match?
Python	`Python 3.12.12`	`Python 3.12.3`	❌ NO
Node.js	`v24.14.0`	`v20.20.0`	❌ NO
Go	`go1.22.12`	`go1.22.12`	✅ YES

Result: ❌ Not all runtimes match — Python and Node.js versions differ between host and chroot environments.

Tested by Smoke Chroot for issue #1161

Copilot AI review requested due to automatic review settings March 5, 2026 19:56

Copilot AI reviewed Mar 5, 2026

View reviewed changes

github-actions bot added the build-test-bun label Mar 5, 2026

github-actions bot added the build-test-go label Mar 5, 2026

github-actions bot mentioned this pull request Mar 5, 2026

[agentics] No-Op Runs #1086

Open

github-actions bot added the smoke-claude label Mar 5, 2026

github-actions bot added build-test-dotnet build-test-deno labels Mar 5, 2026

github-actions bot added the build-test-cpp label Mar 5, 2026

github-actions bot added build-test-rust smoke-copilot labels Mar 5, 2026

github-actions bot added the build-test-node label Mar 5, 2026

github-actions bot added the build-test-java label Mar 5, 2026

	'Failed to generate SSL Bump CA: unexpected string rejection'
	/Failed to generate SSL Bump CA[\s\S]*unexpected string rejection/

Conversation

Mossaka commented Mar 5, 2026

Summary

Test plan

Uh oh!

github-actions bot commented Mar 5, 2026

✅ Coverage Check Passed

Overall Coverage

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 5, 2026

🧪 Build Test: Bun

Uh oh!

github-actions bot commented Mar 5, 2026

Uh oh!

github-actions bot commented Mar 5, 2026

Go Build Test Results

Uh oh!

github-actions bot commented Mar 5, 2026

Smoke Test Results

Uh oh!

github-actions bot commented Mar 5, 2026

.NET Build Test Results

Uh oh!

github-actions bot commented Mar 5, 2026

Build Test: Deno ✅

Uh oh!

github-actions bot commented Mar 5, 2026

C++ Build Test Results

Uh oh!

github-actions bot commented Mar 5, 2026

🦀 Rust Build Test Results

Uh oh!

github-actions bot commented Mar 5, 2026

Uh oh!

github-actions bot commented Mar 5, 2026

Node.js Build Test Results ✅

Uh oh!

github-actions bot commented Mar 5, 2026

Java Build Test Results

Uh oh!

github-actions bot commented Mar 5, 2026

Chroot Version Comparison Results

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants