From 339545891e7377486939036904f7b11d25a96799 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 18 May 2026 09:26:04 +0000 Subject: [PATCH 1/2] test: add branch-coverage tests for container-cleanup.ts Cover previously untested branches in container-cleanup.ts: - sanitizeDockerComposeYaml: null/non-object YAML, missing services, array services, service without environment, array-form env entries with no '=' separator - cleanup(): cli-proxy log preservation (with/without proxyLogsDir), api-proxy-logs inside proxyLogsDir, sessionStateDir specified but missing, auditDir specified but missing, empty default audit dir, unmountSslTmpfs invocation when ssl/ dir exists Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/container-cleanup-branches.test.ts | 340 +++++++++++++++++++++++++ 1 file changed, 340 insertions(+) create mode 100644 src/container-cleanup-branches.test.ts diff --git a/src/container-cleanup-branches.test.ts b/src/container-cleanup-branches.test.ts new file mode 100644 index 00000000..ebb57a14 --- /dev/null +++ b/src/container-cleanup-branches.test.ts @@ -0,0 +1,340 @@ +/** + * Targeted branch-coverage tests for container-cleanup.ts. + * + * These tests cover paths that were not exercised by the existing + * docker-manager-cleanup.test.ts suite, focusing on: + * - sanitizeDockerComposeYaml edge cases + * - cleanup() branches for cli-proxy logs, audit dir, session state, and SSL + */ + +import { cleanup, collectDiagnosticLogs } from './docker-manager'; +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; + +// Mock execa +import { mockExecaFn, mockExecaSync } from './test-helpers/mock-execa.test-utils'; +// eslint-disable-next-line @typescript-eslint/no-require-imports +jest.mock('execa', () => require('./test-helpers/mock-execa.test-utils').execaMockFactory()); + +// Mock ssl-bump so cleanup() doesn't attempt real mount operations +jest.mock('./ssl-bump', () => ({ + cleanupSslKeyMaterial: jest.fn(), + unmountSslTmpfs: jest.fn().mockResolvedValue(undefined), +})); + +jest.mock('./host-env', () => { + const actual = jest.requireActual('./host-env'); + return { + ...actual, + getSafeHostUid: () => String(process.getuid?.() ?? 1000), + getSafeHostGid: () => String(process.getgid?.() ?? 1000), + }; +}); + +// ─── helpers ───────────────────────────────────────────────────────────────── + +function makeTmpDir(): string { + return fs.mkdtempSync(path.join(os.tmpdir(), 'awf-')); +} + +// ─── sanitizeDockerComposeYaml edge cases (via collectDiagnosticLogs) ──────── + +describe('sanitizeDockerComposeYaml edge cases', () => { + let testDir: string; + + beforeEach(() => { + testDir = makeTmpDir(); + jest.clearAllMocks(); + mockExecaFn.mockResolvedValue({ stdout: '', stderr: '', exitCode: 0 }); + }); + + afterEach(() => { + if (fs.existsSync(testDir)) { + fs.rmSync(testDir, { recursive: true, force: true }); + } + }); + + it('returns raw string when YAML parses to a non-object (null)', async () => { + // "null" is valid YAML that parses to null — the sanitizer should return the raw content + fs.writeFileSync(path.join(testDir, 'docker-compose.yml'), 'null'); + await collectDiagnosticLogs(testDir); + const diagnosticsDir = path.join(testDir, 'diagnostics'); + const sanitized = fs.readFileSync(path.join(diagnosticsDir, 'docker-compose.yml'), 'utf8'); + expect(sanitized).toBe('null'); + }); + + it('sanitizes when compose has no services key', async () => { + // Parsed object but without a "services" key — should dump the yaml without error + fs.writeFileSync(path.join(testDir, 'docker-compose.yml'), 'version: "3"\n'); + await collectDiagnosticLogs(testDir); + const diagnosticsDir = path.join(testDir, 'diagnostics'); + expect(fs.existsSync(path.join(diagnosticsDir, 'docker-compose.yml'))).toBe(true); + }); + + it('sanitizes when services is an array instead of an object', async () => { + // services is an array — should be treated as "no services to sanitize" + fs.writeFileSync( + path.join(testDir, 'docker-compose.yml'), + ['version: "3"', 'services:', ' - name: agent'].join('\n') + ); + await collectDiagnosticLogs(testDir); + const diagnosticsDir = path.join(testDir, 'diagnostics'); + expect(fs.existsSync(path.join(diagnosticsDir, 'docker-compose.yml'))).toBe(true); + }); + + it('skips service entries that are not plain objects', async () => { + // A service entry whose value is a primitive (null/string) — should not throw + const raw = ['services:', ' broken_service: null', ' valid_service:', ' image: nginx'].join('\n'); + fs.writeFileSync(path.join(testDir, 'docker-compose.yml'), raw); + await collectDiagnosticLogs(testDir); + const diagnosticsDir = path.join(testDir, 'diagnostics'); + expect(fs.existsSync(path.join(diagnosticsDir, 'docker-compose.yml'))).toBe(true); + }); + + it('preserves all env vars when service has no environment key', async () => { + // Service without an "environment" field — no redaction needed + const raw = ['services:', ' agent:', ' image: ubuntu:22.04'].join('\n'); + fs.writeFileSync(path.join(testDir, 'docker-compose.yml'), raw); + await collectDiagnosticLogs(testDir); + const sanitized = fs.readFileSync( + path.join(testDir, 'diagnostics', 'docker-compose.yml'), + 'utf8' + ); + expect(sanitized).not.toContain('[REDACTED]'); + }); + + it('redacts secrets in array-form environment entries', async () => { + // Array-style environment (list of KEY=VALUE strings) + const raw = [ + 'services:', + ' agent:', + ' environment:', + ' - GITHUB_TOKEN=ghp_array', + ' - NORMAL_VAR=keep_me', + ' - NO_EQUALS_HERE', + ].join('\n'); + fs.writeFileSync(path.join(testDir, 'docker-compose.yml'), raw); + await collectDiagnosticLogs(testDir); + const sanitized = fs.readFileSync( + path.join(testDir, 'diagnostics', 'docker-compose.yml'), + 'utf8' + ); + expect(sanitized).not.toContain('ghp_array'); + expect(sanitized).toContain('keep_me'); + // Entry without "=" should be preserved unchanged + expect(sanitized).toContain('NO_EQUALS_HERE'); + }); +}); + +// ─── cleanup() missing branch coverage ─────────────────────────────────────── + +describe('cleanup - cli-proxy logs', () => { + let testDir: string; + + beforeEach(() => { + testDir = makeTmpDir(); + jest.clearAllMocks(); + mockExecaSync.mockReturnValue(undefined); + }); + + afterEach(() => { + if (fs.existsSync(testDir)) { + fs.rmSync(testDir, { recursive: true, force: true }); + } + }); + + it('chmods cli-proxy-logs inside proxyLogsDir when it exists', async () => { + const proxyLogsDir = fs.mkdtempSync(path.join(os.tmpdir(), 'awf-proxy-')); + try { + const cliProxyLogsDir = path.join(proxyLogsDir, 'cli-proxy-logs'); + fs.mkdirSync(cliProxyLogsDir, { recursive: true }); + fs.writeFileSync(path.join(cliProxyLogsDir, 'difc-proxy.log'), 'audit entry\n'); + + await cleanup(testDir, false, proxyLogsDir); + + expect(mockExecaSync).toHaveBeenCalledWith('chmod', ['-R', 'a+rX', cliProxyLogsDir]); + } finally { + if (fs.existsSync(proxyLogsDir)) { + fs.rmSync(proxyLogsDir, { recursive: true, force: true }); + } + } + }); + + it('moves non-empty cli-proxy-logs to /tmp when proxyLogsDir is not specified', async () => { + const cliProxyLogsDir = path.join(testDir, 'cli-proxy-logs'); + fs.mkdirSync(cliProxyLogsDir, { recursive: true }); + fs.writeFileSync(path.join(cliProxyLogsDir, 'difc-proxy.log'), 'audit entry\n'); + + await cleanup(testDir, false); + + const timestamp = path.basename(testDir).replace('awf-', ''); + const destination = path.join(os.tmpdir(), `cli-proxy-logs-${timestamp}`); + expect(fs.existsSync(destination)).toBe(true); + // testDir is deleted by cleanup; clean up the destination + if (fs.existsSync(destination)) { + fs.rmSync(destination, { recursive: true, force: true }); + } + }); + + it('does not move empty cli-proxy-logs directory', async () => { + const cliProxyLogsDir = path.join(testDir, 'cli-proxy-logs'); + fs.mkdirSync(cliProxyLogsDir, { recursive: true }); + // leave it empty + + await cleanup(testDir, false); + + const timestamp = path.basename(testDir).replace('awf-', ''); + const destination = path.join(os.tmpdir(), `cli-proxy-logs-${timestamp}`); + expect(fs.existsSync(destination)).toBe(false); + }); +}); + +describe('cleanup - api-proxy logs via proxyLogsDir', () => { + let testDir: string; + + beforeEach(() => { + testDir = makeTmpDir(); + jest.clearAllMocks(); + mockExecaSync.mockReturnValue(undefined); + }); + + afterEach(() => { + if (fs.existsSync(testDir)) { + fs.rmSync(testDir, { recursive: true, force: true }); + } + }); + + it('chmods api-proxy-logs inside proxyLogsDir when it exists and is non-empty', async () => { + const proxyLogsDir = fs.mkdtempSync(path.join(os.tmpdir(), 'awf-proxy-')); + try { + const apiProxyLogsDir = path.join(proxyLogsDir, 'api-proxy-logs'); + fs.mkdirSync(apiProxyLogsDir, { recursive: true }); + fs.writeFileSync(path.join(apiProxyLogsDir, 'proxy.log'), 'request\n'); + + await cleanup(testDir, false, proxyLogsDir); + + expect(mockExecaSync).toHaveBeenCalledWith('chmod', ['-R', 'a+rX', apiProxyLogsDir]); + } finally { + if (fs.existsSync(proxyLogsDir)) { + fs.rmSync(proxyLogsDir, { recursive: true, force: true }); + } + } + }); +}); + +describe('cleanup - audit dir', () => { + let testDir: string; + + beforeEach(() => { + testDir = makeTmpDir(); + jest.clearAllMocks(); + mockExecaSync.mockReturnValue(undefined); + }); + + afterEach(() => { + if (fs.existsSync(testDir)) { + fs.rmSync(testDir, { recursive: true, force: true }); + } + }); + + it('skips chmod when auditDir is specified but does not exist', async () => { + const nonExistentAuditDir = path.join(os.tmpdir(), `awf-nonexistent-audit-${Date.now()}`); + + await cleanup(testDir, false, undefined, nonExistentAuditDir); + + expect(mockExecaSync).not.toHaveBeenCalledWith('chmod', ['-R', 'a+rX', nonExistentAuditDir]); + }); + + it('does not move empty default audit directory', async () => { + const defaultAuditDir = path.join(testDir, 'audit'); + fs.mkdirSync(defaultAuditDir, { recursive: true }); + // leave it empty + + await cleanup(testDir, false); + + const timestamp = path.basename(testDir).replace('awf-', ''); + const destination = path.join(os.tmpdir(), `awf-audit-${timestamp}`); + expect(fs.existsSync(destination)).toBe(false); + }); +}); + +describe('cleanup - sessionStateDir', () => { + let testDir: string; + + beforeEach(() => { + testDir = makeTmpDir(); + jest.clearAllMocks(); + mockExecaSync.mockReturnValue(undefined); + }); + + afterEach(() => { + if (fs.existsSync(testDir)) { + fs.rmSync(testDir, { recursive: true, force: true }); + } + }); + + it('skips chmod when sessionStateDir is specified but does not exist', async () => { + const nonExistentStateDir = path.join(os.tmpdir(), `awf-nonexistent-state-${Date.now()}`); + + await cleanup(testDir, false, undefined, undefined, nonExistentStateDir); + + expect(mockExecaSync).not.toHaveBeenCalledWith('chmod', ['-R', 'a+rX', nonExistentStateDir]); + }); + + it('chmods sessionStateDir in-place when it exists', async () => { + const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), 'awf-state-')); + try { + fs.writeFileSync(path.join(stateDir, 'events.jsonl'), '{"type":"start"}\n'); + + await cleanup(testDir, false, undefined, undefined, stateDir); + + expect(mockExecaSync).toHaveBeenCalledWith('chmod', ['-R', 'a+rX', stateDir]); + } finally { + if (fs.existsSync(stateDir)) { + fs.rmSync(stateDir, { recursive: true, force: true }); + } + } + }); +}); + +describe('cleanup - SSL directory', () => { + let testDir: string; + + beforeEach(() => { + testDir = makeTmpDir(); + jest.clearAllMocks(); + mockExecaSync.mockReturnValue(undefined); + }); + + afterEach(() => { + if (fs.existsSync(testDir)) { + fs.rmSync(testDir, { recursive: true, force: true }); + } + }); + + it('calls unmountSslTmpfs when ssl directory exists in workDir', async () => { + const { unmountSslTmpfs } = jest.requireMock('./ssl-bump') as { + unmountSslTmpfs: jest.Mock; + }; + unmountSslTmpfs.mockResolvedValue(undefined); + + const sslDir = path.join(testDir, 'ssl'); + fs.mkdirSync(sslDir, { recursive: true }); + fs.writeFileSync(path.join(sslDir, 'ca.pem'), 'fake-cert'); + + await cleanup(testDir, false); + + expect(unmountSslTmpfs).toHaveBeenCalledWith(sslDir); + }); + + it('does not call unmountSslTmpfs when ssl directory does not exist', async () => { + const { unmountSslTmpfs } = jest.requireMock('./ssl-bump') as { + unmountSslTmpfs: jest.Mock; + }; + + await cleanup(testDir, false); + + expect(unmountSslTmpfs).not.toHaveBeenCalled(); + }); +}); From f2a36bc0a8937f0e2fbe4d79d7292bef99bc0222 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 18 May 2026 14:52:24 +0000 Subject: [PATCH 2/2] test: address container cleanup review feedback --- src/container-cleanup-branches.test.ts | 33 ++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/src/container-cleanup-branches.test.ts b/src/container-cleanup-branches.test.ts index ebb57a14..a6d78aa4 100644 --- a/src/container-cleanup-branches.test.ts +++ b/src/container-cleanup-branches.test.ts @@ -7,10 +7,11 @@ * - cleanup() branches for cli-proxy logs, audit dir, session state, and SSL */ -import { cleanup, collectDiagnosticLogs } from './docker-manager'; +import { cleanup, collectDiagnosticLogs } from './container-cleanup'; import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; +import * as yaml from 'js-yaml'; // Mock execa import { mockExecaFn, mockExecaSync } from './test-helpers/mock-execa.test-utils'; @@ -69,7 +70,8 @@ describe('sanitizeDockerComposeYaml edge cases', () => { fs.writeFileSync(path.join(testDir, 'docker-compose.yml'), 'version: "3"\n'); await collectDiagnosticLogs(testDir); const diagnosticsDir = path.join(testDir, 'diagnostics'); - expect(fs.existsSync(path.join(diagnosticsDir, 'docker-compose.yml'))).toBe(true); + const sanitized = fs.readFileSync(path.join(diagnosticsDir, 'docker-compose.yml'), 'utf8'); + expect(yaml.load(sanitized)).toEqual({ version: '3' }); }); it('sanitizes when services is an array instead of an object', async () => { @@ -80,7 +82,8 @@ describe('sanitizeDockerComposeYaml edge cases', () => { ); await collectDiagnosticLogs(testDir); const diagnosticsDir = path.join(testDir, 'diagnostics'); - expect(fs.existsSync(path.join(diagnosticsDir, 'docker-compose.yml'))).toBe(true); + const sanitized = fs.readFileSync(path.join(diagnosticsDir, 'docker-compose.yml'), 'utf8'); + expect(yaml.load(sanitized)).toEqual({ version: '3', services: [{ name: 'agent' }] }); }); it('skips service entries that are not plain objects', async () => { @@ -89,7 +92,15 @@ describe('sanitizeDockerComposeYaml edge cases', () => { fs.writeFileSync(path.join(testDir, 'docker-compose.yml'), raw); await collectDiagnosticLogs(testDir); const diagnosticsDir = path.join(testDir, 'diagnostics'); - expect(fs.existsSync(path.join(diagnosticsDir, 'docker-compose.yml'))).toBe(true); + const sanitized = fs.readFileSync(path.join(diagnosticsDir, 'docker-compose.yml'), 'utf8'); + expect(yaml.load(sanitized)).toEqual({ + services: { + broken_service: null, + valid_service: { + image: 'nginx', + }, + }, + }); }); it('preserves all env vars when service has no environment key', async () => { @@ -102,6 +113,13 @@ describe('sanitizeDockerComposeYaml edge cases', () => { 'utf8' ); expect(sanitized).not.toContain('[REDACTED]'); + expect(yaml.load(sanitized)).toEqual({ + services: { + agent: { + image: 'ubuntu:22.04', + }, + }, + }); }); it('redacts secrets in array-form environment entries', async () => { @@ -171,6 +189,9 @@ describe('cleanup - cli-proxy logs', () => { const timestamp = path.basename(testDir).replace('awf-', ''); const destination = path.join(os.tmpdir(), `cli-proxy-logs-${timestamp}`); expect(fs.existsSync(destination)).toBe(true); + const movedLogPath = path.join(destination, 'difc-proxy.log'); + expect(fs.existsSync(movedLogPath)).toBe(true); + expect(fs.readFileSync(movedLogPath, 'utf8')).toBe('audit entry\n'); // testDir is deleted by cleanup; clean up the destination if (fs.existsSync(destination)) { fs.rmSync(destination, { recursive: true, force: true }); @@ -314,7 +335,8 @@ describe('cleanup - SSL directory', () => { }); it('calls unmountSslTmpfs when ssl directory exists in workDir', async () => { - const { unmountSslTmpfs } = jest.requireMock('./ssl-bump') as { + const { cleanupSslKeyMaterial, unmountSslTmpfs } = jest.requireMock('./ssl-bump') as { + cleanupSslKeyMaterial: jest.Mock; unmountSslTmpfs: jest.Mock; }; unmountSslTmpfs.mockResolvedValue(undefined); @@ -325,6 +347,7 @@ describe('cleanup - SSL directory', () => { await cleanup(testDir, false); + expect(cleanupSslKeyMaterial).toHaveBeenCalledWith(testDir); expect(unmountSslTmpfs).toHaveBeenCalledWith(sslDir); });