Properly clean up tmp files

Author: JessRudder

__tests__/main.test.ts

@@ -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)
+  })
 })

dist/index.js

@@ -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