Properly clean up tmp files

This commit is contained in:
Jessica Rudder
2025-08-12 14:25:08 -07:00
parent 3ba8e1b39d
commit a2fd223fcf
4 changed files with 85 additions and 5 deletions

View File

@@ -75,6 +75,19 @@ vi.mock('fs', () => ({
writeFileSync: mockWriteFileSync,
}))
// Mocks for tmp module to control temporary file creation and cleanup
const mockRemoveCallback = vi.fn()
const mockFileSync = vi.fn().mockReturnValue({
name: '/secure/temp/dir/modelResponse-abc123.txt',
removeCallback: mockRemoveCallback,
})
const mockSetGracefulCleanup = vi.fn()
vi.mock('tmp', () => ({
fileSync: mockFileSync,
setGracefulCleanup: mockSetGracefulCleanup,
}))
// Mock MCP and inference modules
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const mockConnectToGitHubMCP = vi.fn() as MockedFunction<any>
@@ -269,4 +282,43 @@ describe('main.ts', () => {
expect(core.setFailed).toHaveBeenCalledWith(`File for prompt-file was not found: ${promptFile}`)
expect(mockProcessExit).toHaveBeenCalledWith(1)
})
it('creates secure temporary files with proper cleanup', async () => {
mockInputs({
prompt: 'Test prompt',
'system-prompt': 'You are a test assistant.',
})
await run()
expect(mockSetGracefulCleanup).toHaveBeenCalledOnce()
expect(mockFileSync).toHaveBeenCalledWith({
prefix: 'modelResponse-',
postfix: '.txt',
})
expect(core.setOutput).toHaveBeenNthCalledWith(2, 'response-file', '/secure/temp/dir/modelResponse-abc123.txt')
expect(mockWriteFileSync).toHaveBeenCalledWith('/secure/temp/dir/modelResponse-abc123.txt', 'Hello, user!', 'utf-8')
expect(mockRemoveCallback).toHaveBeenCalledOnce()
expect(mockProcessExit).toHaveBeenCalledWith(0)
})
it('handles cleanup errors gracefully', async () => {
mockRemoveCallback.mockImplementationOnce(() => {
throw new Error('Cleanup failed')
})
mockInputs({
prompt: 'Test prompt',
'system-prompt': 'You are a test assistant.',
})
await run()
expect(mockRemoveCallback).toHaveBeenCalledOnce()
expect(core.warning).toHaveBeenCalledWith('Failed to cleanup temporary file: Error: Cleanup failed')
expect(mockProcessExit).toHaveBeenCalledWith(0)
})
})

18
dist/index.js generated vendored
View File

@@ -52596,6 +52596,9 @@ function isPromptYamlFile(filePath) {
* @returns Resolves when the action is complete.
*/
async function run() {
let responseFile = null;
// Set up graceful cleanup for temporary files on process exit
tmpExports.setGracefulCleanup();
try {
const promptFilePath = coreExports.getInput('prompt-file');
const inputVariables = coreExports.getInput('input');
@@ -52648,10 +52651,9 @@ async function run() {
}
coreExports.setOutput('response', modelResponse || '');
// Create a secure temporary file instead of using the temp directory directly
const responseFile = tmpExports.fileSync({
responseFile = tmpExports.fileSync({
prefix: 'modelResponse-',
postfix: '.txt',
keep: true, // Keep the file so the action can read it
});
coreExports.setOutput('response-file', responseFile.name);
if (modelResponse && modelResponse !== '') {
@@ -52668,6 +52670,18 @@ async function run() {
// Force exit to prevent hanging on open connections
process.exit(1);
}
finally {
// Explicit cleanup of temporary file if it was created
if (responseFile) {
try {
responseFile.removeCallback();
}
catch (cleanupError) {
// Log cleanup errors but don't fail the action
coreExports.warning(`Failed to cleanup temporary file: ${cleanupError}`);
}
}
}
// Force exit to prevent hanging on open connections
process.exit(0);
}

2
dist/index.js.map generated vendored

File diff suppressed because one or more lines are too long

View File

@@ -18,6 +18,11 @@ import {
* @returns Resolves when the action is complete.
*/
export async function run(): Promise<void> {
let responseFile: tmp.FileResult | null = null
// Set up graceful cleanup for temporary files on process exit
tmp.setGracefulCleanup()
try {
const promptFilePath = core.getInput('prompt-file')
const inputVariables = core.getInput('input')
@@ -91,10 +96,9 @@ export async function run(): Promise<void> {
core.setOutput('response', modelResponse || '')
// Create a secure temporary file instead of using the temp directory directly
const responseFile = tmp.fileSync({
responseFile = tmp.fileSync({
prefix: 'modelResponse-',
postfix: '.txt',
keep: true, // Keep the file so the action can read it
})
core.setOutput('response-file', responseFile.name)
@@ -110,6 +114,16 @@ export async function run(): Promise<void> {
}
// Force exit to prevent hanging on open connections
process.exit(1)
} finally {
// Explicit cleanup of temporary file if it was created
if (responseFile) {
try {
responseFile.removeCallback()
} catch (cleanupError) {
// Log cleanup errors but don't fail the action
core.warning(`Failed to cleanup temporary file: ${cleanupError}`)
}
}
}
// Force exit to prevent hanging on open connections