Skip to content

Commit f347eae

Browse files
authored
Merge pull request #91 from JessRudder/secure-tmp-files
Uses tmp library to ensure more secure tmp file creation
2 parents c72cb2e + 07fe2f3 commit f347eae

File tree

8 files changed

+1068
-53
lines changed

8 files changed

+1068
-53
lines changed

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

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

.licenses/npm/tmp.dep.yml

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

__tests__/main.test.ts

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ function mockInputs(inputs: Record<string, string> = {}): void {
6666
*/
6767
function verifyStandardResponse(): void {
6868
expect(core.setOutput).toHaveBeenNthCalledWith(1, 'response', 'Hello, user!')
69-
expect(core.setOutput).toHaveBeenNthCalledWith(2, 'response-file', expect.stringContaining('modelResponse.txt'))
69+
expect(core.setOutput).toHaveBeenNthCalledWith(2, 'response-file', expect.stringContaining('modelResponse-'))
7070
}
7171

7272
vi.mock('fs', () => ({
@@ -75,6 +75,19 @@ vi.mock('fs', () => ({
7575
writeFileSync: mockWriteFileSync,
7676
}))
7777

78+
// Mocks for tmp module to control temporary file creation and cleanup
79+
const mockRemoveCallback = vi.fn()
80+
const mockFileSync = vi.fn().mockReturnValue({
81+
name: '/secure/temp/dir/modelResponse-abc123.txt',
82+
removeCallback: mockRemoveCallback,
83+
})
84+
const mockSetGracefulCleanup = vi.fn()
85+
86+
vi.mock('tmp', () => ({
87+
fileSync: mockFileSync,
88+
setGracefulCleanup: mockSetGracefulCleanup,
89+
}))
90+
7891
// Mock MCP and inference modules
7992
// eslint-disable-next-line @typescript-eslint/no-explicit-any
8093
const mockConnectToGitHubMCP = vi.fn() as MockedFunction<any>
@@ -269,4 +282,43 @@ describe('main.ts', () => {
269282
expect(core.setFailed).toHaveBeenCalledWith(`File for prompt-file was not found: ${promptFile}`)
270283
expect(mockProcessExit).toHaveBeenCalledWith(1)
271284
})
285+
286+
it('creates secure temporary files with proper cleanup', async () => {
287+
mockInputs({
288+
prompt: 'Test prompt',
289+
'system-prompt': 'You are a test assistant.',
290+
})
291+
292+
await run()
293+
294+
expect(mockSetGracefulCleanup).toHaveBeenCalledOnce()
295+
296+
expect(mockFileSync).toHaveBeenCalledWith({
297+
prefix: 'modelResponse-',
298+
postfix: '.txt',
299+
})
300+
301+
expect(core.setOutput).toHaveBeenNthCalledWith(2, 'response-file', '/secure/temp/dir/modelResponse-abc123.txt')
302+
expect(mockWriteFileSync).toHaveBeenCalledWith('/secure/temp/dir/modelResponse-abc123.txt', 'Hello, user!', 'utf-8')
303+
expect(mockRemoveCallback).toHaveBeenCalledOnce()
304+
305+
expect(mockProcessExit).toHaveBeenCalledWith(0)
306+
})
307+
308+
it('handles cleanup errors gracefully', async () => {
309+
mockRemoveCallback.mockImplementationOnce(() => {
310+
throw new Error('Cleanup failed')
311+
})
312+
313+
mockInputs({
314+
prompt: 'Test prompt',
315+
'system-prompt': 'You are a test assistant.',
316+
})
317+
318+
await run()
319+
320+
expect(mockRemoveCallback).toHaveBeenCalledOnce()
321+
expect(core.warning).toHaveBeenCalledWith('Failed to cleanup temporary file: Error: Cleanup failed')
322+
expect(mockProcessExit).toHaveBeenCalledWith(0)
323+
})
272324
})

0 commit comments

Comments
 (0)