Merge pull request #91 from JessRudder/secure-tmp-files

Uses tmp library to ensure more secure tmp file creation

.github/workflows/licensed.yml

@@ -42,11 +42,11 @@ jobs:
 
       - name: Setup Ruby
         id: setup-ruby
-        uses: ruby/setup-ruby@v1
+        uses: ruby/setup-ruby@829114fc20da43a41d27359103ec7a63020954d4
         with:
           ruby-version: ruby
 
-      - uses: licensee/setup-licensed@v1.3.2
+      - uses: licensee/setup-licensed@0d52e575b3258417672be0dff2f115d7db8771d8
         with:
           version: 4.x
           github_token: ${{ secrets.GITHUB_TOKEN }}

.licenses/npm/@types/tmp.dep.yml

@@ -0,0 +1,32 @@
+---
+name: "@types/tmp"
+version: 0.2.6
+type: npm
+summary: TypeScript definitions for tmp
+homepage: https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/tmp
+license: mit
+licenses:
+- sources: LICENSE
+  text: |2
+        MIT License
+
+        Copyright (c) Microsoft Corporation.
+
+        Permission is hereby granted, free of charge, to any person obtaining a copy
+        of this software and associated documentation files (the "Software"), to deal
+        in the Software without restriction, including without limitation the rights
+        to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+        copies of the Software, and to permit persons to whom the Software is
+        furnished to do so, subject to the following conditions:
+
+        The above copyright notice and this permission notice shall be included in all
+        copies or substantial portions of the Software.
+
+        THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+        IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+        FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+        AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+        LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+        OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+        SOFTWARE
+notices: []

.licenses/npm/tmp.dep.yml

@@ -0,0 +1,32 @@
+---
+name: tmp
+version: 0.2.5
+type: npm
+summary: Temporary file and directory creator
+homepage: http://github.com/raszi/node-tmp
+license: mit
+licenses:
+- sources: LICENSE
+  text: |
+    The MIT License (MIT)
+
+    Copyright (c) 2014 KARASZI István
+
+    Permission is hereby granted, free of charge, to any person obtaining a copy
+    of this software and associated documentation files (the "Software"), to deal
+    in the Software without restriction, including without limitation the rights
+    to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+    copies of the Software, and to permit persons to whom the Software is
+    furnished to do so, subject to the following conditions:
+
+    The above copyright notice and this permission notice shall be included in all
+    copies or substantial portions of the Software.
+
+    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+    IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+    AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+    LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+    OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+    SOFTWARE.
+notices: []

.node-version

@@ -1 +1 @@
-20.9.0
+24.4.0

__tests__/main.test.ts

@@ -66,7 +66,7 @@ function mockInputs(inputs: Record<string, string> = {}): void {
  */
 function verifyStandardResponse(): void {
   expect(core.setOutput).toHaveBeenNthCalledWith(1, 'response', 'Hello, user!')
-  expect(core.setOutput).toHaveBeenNthCalledWith(2, 'response-file', expect.stringContaining('modelResponse.txt'))
+  expect(core.setOutput).toHaveBeenNthCalledWith(2, 'response-file', expect.stringContaining('modelResponse-'))
 }
 
 vi.mock('fs', () => ({
@@ -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)
+  })
 })

action.yml

@@ -67,5 +67,5 @@ outputs:
     description: The file path where the response is saved
 
 runs:
-  using: node20
+  using: node24
   main: dist/index.js

package-lock.json

@@ -11,9 +11,11 @@
       "dependencies": {
         "@actions/core": "^1.11.1",
         "@modelcontextprotocol/sdk": "^1.15.1",
+        "@types/tmp": "^0.2.6",
         "js-yaml": "^4.1.0",
         "openai": "^5.11.0",
-        "pkce-challenge": "^5.0.0"
+        "pkce-challenge": "^5.0.0",
+        "tmp": "^0.2.4"
       },
       "devDependencies": {
         "@eslint/compat": "^1.3.0",
@@ -24,7 +26,7 @@
         "@rollup/plugin-node-resolve": "^16.0.1",
         "@rollup/plugin-typescript": "^12.1.2",
         "@types/js-yaml": "^4.0.9",
-        "@types/node": "^22.15.31",
+        "@types/node": "^24.1.0",
         "@typescript-eslint/eslint-plugin": "^8.34.0",
         "@typescript-eslint/parser": "^8.32.1",
         "eslint": "^9.29.0",
@@ -39,7 +41,7 @@
         "vitest": "^3"
       },
       "engines": {
-        "node": ">=20"
+        "node": ">=24"
       },
       "optionalDependencies": {
         "@rollup/rollup-linux-x64-gnu": "*"
@@ -2476,13 +2478,13 @@
       "license": "MIT"
     },
     "node_modules/@types/node": {
-      "version": "22.15.31",
-      "resolved": "https://registry.npmjs.org/@types/node/-/node-22.15.31.tgz",
-      "integrity": "sha512-jnVe5ULKl6tijxUhvQeNbQG/84fHfg+yMak02cT8QVhBx/F05rAVxCGBYYTh2EKz22D6JF5ktXuNwdx7b9iEGw==",
+      "version": "24.1.0",
+      "resolved": "https://registry.npmjs.org/@types/node/-/node-24.1.0.tgz",
+      "integrity": "sha512-ut5FthK5moxFKH2T1CUOC6ctR67rQRvvHdFLCD2Ql6KXmMuCrjsSsRI9UsLCm9M18BMwClv4pn327UvB7eeO1w==",
       "dev": true,
       "license": "MIT",
       "dependencies": {
-        "undici-types": "~6.21.0"
+        "undici-types": "~7.8.0"
       }
     },
     "node_modules/@types/resolve": {
@@ -2492,6 +2494,12 @@
       "dev": true,
       "license": "MIT"
     },
+    "node_modules/@types/tmp": {
+      "version": "0.2.6",
+      "resolved": "https://registry.npmjs.org/@types/tmp/-/tmp-0.2.6.tgz",
+      "integrity": "sha512-chhaNf2oKHlRkDGt+tiKE2Z5aJ6qalm7Z9rlLdBwmOiAAf09YQvvoLXjWK4HWPF1xU/fqvMgfNfpVoBscA/tKA==",
+      "license": "MIT"
+    },
     "node_modules/@typescript-eslint/eslint-plugin": {
       "version": "8.34.0",
       "resolved": "https://registry.npmjs.org/@typescript-eslint/eslint-plugin/-/eslint-plugin-8.34.0.tgz",
@@ -8943,6 +8951,15 @@
         "node": ">=14.0.0"
       }
     },
+    "node_modules/tmp": {
+      "version": "0.2.5",
+      "resolved": "https://registry.npmjs.org/tmp/-/tmp-0.2.5.tgz",
+      "integrity": "sha512-voyz6MApa1rQGUxT3E+BK7/ROe8itEx7vD8/HEvt4xwXucvQ5G5oeEiHkmHZJuBO21RpOf+YYm9MOivj709jow==",
+      "license": "MIT",
+      "engines": {
+        "node": ">=14.14"
+      }
+    },
     "node_modules/to-regex-range": {
       "version": "5.0.1",
       "resolved": "https://registry.npmjs.org/to-regex-range/-/to-regex-range-5.0.1.tgz",
@@ -9207,9 +9224,9 @@
       }
     },
     "node_modules/undici-types": {
-      "version": "6.21.0",
-      "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-6.21.0.tgz",
-      "integrity": "sha512-iwDZqg0QAGrg9Rav5H4n0M64c3mkR59cJ6wQp+7C4nI0gsmExaedaYLNO44eT4AtBBwjbTiGPMlt2Md0T9H9JQ==",
+      "version": "7.8.0",
+      "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.8.0.tgz",
+      "integrity": "sha512-9UJ2xGDvQ43tYyVMpuHlsgApydB8ZKfVYTsLDhXkFL/6gfkp+U8xTGdh8pMJv1SpZna0zxG1DwsKZsreLbXBxw==",
       "dev": true,
       "license": "MIT"
     },

package.json

@@ -6,7 +6,7 @@
     ".": "./dist/index.js"
   },
   "engines": {
-    "node": ">=20"
+    "node": ">=24"
   },
   "scripts": {
     "bundle": "npm run format:write && npm run package",
@@ -25,9 +25,11 @@
   "dependencies": {
     "@actions/core": "^1.11.1",
     "@modelcontextprotocol/sdk": "^1.15.1",
+    "@types/tmp": "^0.2.6",
     "js-yaml": "^4.1.0",
     "openai": "^5.11.0",
-    "pkce-challenge": "^5.0.0"
+    "pkce-challenge": "^5.0.0",
+    "tmp": "^0.2.4"
   },
   "devDependencies": {
     "@eslint/compat": "^1.3.0",
@@ -38,7 +40,7 @@
     "@rollup/plugin-node-resolve": "^16.0.1",
     "@rollup/plugin-typescript": "^12.1.2",
     "@types/js-yaml": "^4.0.9",
-    "@types/node": "^22.15.31",
+    "@types/node": "^24.1.0",
     "@typescript-eslint/eslint-plugin": "^8.34.0",
     "@typescript-eslint/parser": "^8.32.1",
     "eslint": "^9.29.0",

src/main.ts

@@ -1,7 +1,6 @@
 import * as core from '@actions/core'
 import * as fs from 'fs'
-import * as os from 'os'
-import * as path from 'path'
+import * as tmp from 'tmp'
 import {connectToGitHubMCP} from './mcp.js'
 import {simpleInference, mcpInference} from './inference.js'
 import {loadContentFromFileOrInput, buildInferenceRequest} from './helpers.js'
@@ -13,14 +12,17 @@ import {
   parseFileTemplateVariables,
 } from './prompt.js'
 
-const RESPONSE_FILE = 'modelResponse.txt'
-
 /**
  * The main function for the action.
  *
  * @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')
@@ -93,11 +95,16 @@ export async function run(): Promise<void> {
 
     core.setOutput('response', modelResponse || '')
 
-    const responseFilePath = path.join(tempDir(), RESPONSE_FILE)
-    core.setOutput('response-file', responseFilePath)
+    // Create a secure temporary file instead of using the temp directory directly
+    responseFile = tmp.fileSync({
+      prefix: 'modelResponse-',
+      postfix: '.txt',
+    })
+
+    core.setOutput('response-file', responseFile.name)
 
     if (modelResponse && modelResponse !== '') {
-      fs.writeFileSync(responseFilePath, modelResponse, 'utf-8')
+      fs.writeFileSync(responseFile.name, modelResponse, 'utf-8')
     }
   } catch (error) {
     if (error instanceof Error) {
@@ -107,13 +114,18 @@ 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
   process.exit(0)
 }
-
-function tempDir(): string {
-  const tempDirectory = process.env['RUNNER_TEMP'] || os.tmpdir()
-  return tempDirectory
-}