Add config file support for tsu collate with per-command timeout settings#173
Add config file support for tsu collate with per-command timeout settings#173
Conversation
Co-authored-by: bestdan <2766380+bestdan@users.noreply.github.com>
Co-authored-by: bestdan <2766380+bestdan@users.noreply.github.com>
Co-authored-by: bestdan <2766380+bestdan@users.noreply.github.com>
|
|
||
| it('should load .tsurc file', () => { | ||
| const configData: TsuConfig = { timeout: 5000 }; | ||
| writeFileSync(join(testDir, '.tsurc'), JSON.stringify(configData)); |
Check failure
Code scanning / CodeQL
Insecure temporary file High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
In general, to fix insecure temporary file/directory creation, avoid manually constructing paths under os.tmpdir() using timestamps or Math.random(). Instead, use a library designed for secure temporary resources (such as tmp) or Node’s fs.mkdtemp, which creates a uniquely named directory with appropriate checks to avoid races and collisions. Within that directory, you can then create files normally; the key is that the directory itself was securely created.
For this specific test file, the best fix is to stop manually joining tmpdir() with a custom name in beforeEach, and instead use fs.mkdtempSync with a prefix under tmpdir(). That keeps the behavior (one isolated temp directory per test, removed in afterEach) while ensuring the directory name is unique and created atomically. Concretely:
- Import
mkdtempSyncfromnode:fs. - In
beforeEach, compute a base temp directory prefix withjoin(tmpdir(), 'tsu-test-'), then callmkdtempSyncwith that prefix and assign the resulting unique directory path totestDir. - Leave the rest of the tests unchanged, since they already use
testDirfor writing files and will thus benefit from the more secure temp directory.
All changes are confined to src/utils/config.test.ts, specifically to the imports and the beforeEach setup of testDir.
| @@ -1,5 +1,5 @@ | ||
| import { describe, it, expect, beforeEach, afterEach } from 'vitest'; | ||
| import { writeFileSync, mkdirSync, rmSync } from 'node:fs'; | ||
| import { writeFileSync, mkdirSync, rmSync, mkdtempSync } from 'node:fs'; | ||
| import { join } from 'node:path'; | ||
| import { tmpdir } from 'node:os'; | ||
| import { loadConfig, getTimeoutFromConfig } from './config.js'; | ||
| @@ -9,9 +9,9 @@ | ||
| let testDir: string; | ||
|
|
||
| beforeEach(() => { | ||
| // Create a unique temp directory for each test | ||
| testDir = join(tmpdir(), `tsu-test-${Date.now()}-${Math.random().toString(36).slice(2)}`); | ||
| mkdirSync(testDir, { recursive: true }); | ||
| // Create a secure unique temp directory for each test | ||
| const baseTmp = join(tmpdir(), 'tsu-test-'); | ||
| testDir = mkdtempSync(baseTmp); | ||
| }); | ||
|
|
||
| afterEach(() => { |
|
|
||
| it('should load .tsurc.json file', () => { | ||
| const configData: TsuConfig = { timeout: 10000 }; | ||
| writeFileSync(join(testDir, '.tsurc.json'), JSON.stringify(configData)); |
Check failure
Code scanning / CodeQL
Insecure temporary file High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
In general, to fix insecure temporary file/directory creation, avoid manually constructing paths under os.tmpdir() and instead use secure APIs that both guarantee uniqueness and create the directory/file atomically with safe permissions. In Node.js, that typically means using fs.mkdtemp/fs.mkdtempSync (or a library like tmp) with a fixed prefix, letting Node/OS provide the rest of the unique name.
For this file, the simplest, behavior-preserving change is to replace the manual join(tmpdir(), ...) construction of testDir with a call to mkdtempSync using a secure base directory. That gives us a unique temp directory created atomically, while keeping the rest of the test code identical (all the join(testDir, ...) usages continue to work). Concretely:
- Update the imports from
node:fsto includemkdtempSync. - In
beforeEach, replace:- the comment “Create a unique temp directory for each test”
- and the line that manually builds
testDirwithjoin(tmpdir(), ...Date.now()...Math.random...)andmkdirSync(testDir, { recursive: true });
with a singlemkdtempSynccall using a suitable prefix derived fromtmpdir(), e.g.testDir = mkdtempSync(join(tmpdir(), 'tsu-test-'));.
- Keep
afterEachcleanup and all other references totestDirunchanged.
No new helper methods are required; we only need the additional import and the replacement of the directory creation logic.
| @@ -1,5 +1,5 @@ | ||
| import { describe, it, expect, beforeEach, afterEach } from 'vitest'; | ||
| import { writeFileSync, mkdirSync, rmSync } from 'node:fs'; | ||
| import { writeFileSync, mkdirSync, rmSync, mkdtempSync } from 'node:fs'; | ||
| import { join } from 'node:path'; | ||
| import { tmpdir } from 'node:os'; | ||
| import { loadConfig, getTimeoutFromConfig } from './config.js'; | ||
| @@ -9,9 +9,8 @@ | ||
| let testDir: string; | ||
|
|
||
| beforeEach(() => { | ||
| // Create a unique temp directory for each test | ||
| testDir = join(tmpdir(), `tsu-test-${Date.now()}-${Math.random().toString(36).slice(2)}`); | ||
| mkdirSync(testDir, { recursive: true }); | ||
| // Create a secure, unique temp directory for each test | ||
| testDir = mkdtempSync(join(tmpdir(), 'tsu-test-')); | ||
| }); | ||
|
|
||
| afterEach(() => { |
|
|
||
| it('should load tsu.config.json file', () => { | ||
| const configData: TsuConfig = { timeout: 15000 }; | ||
| writeFileSync(join(testDir, 'tsu.config.json'), JSON.stringify(configData)); |
Check failure
Code scanning / CodeQL
Insecure temporary file High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
In general, the issue is best fixed by delegating creation of temporary directories/files to a well-tested library that (1) uses secure permissions, (2) ensures the path does not already exist, and (3) avoids predictable naming. For Node.js, the commonly recommended option is the tmp package, which provides tmp.dirSync() and tmp.fileSync() with secure defaults.
For this specific file, the only tainted element is the use of tmpdir() combined with manually generated directory names in beforeEach. We can replace the manual construction of testDir with a call to tmp.dirSync() from the tmp library. tmp.dirSync() returns an object containing the created directory name and a cleanup callback; however, the tests already clean up with rmSync, so to avoid changing existing behaviour we can use only the .name property and keep the existing afterEach cleanup. No other logic in the tests needs to change, because all uses of testDir are simply for writing or reading config files inside that directory.
Concretely:
- Add an import for
tmpat the top ofsrc/utils/config.test.ts. - Remove the import of
tmpdirfromnode:osbecause it is no longer used. - In
beforeEach, replace the manual construction usingjoin(tmpdir(), ... )andmkdirSyncwith a single call totmp.dirSync({ unsafeCleanup: true }).name. This both creates the directory securely and returns its path;unsafeCleanupmirrors the current behaviour of recursively deleting the directory contents. - Keep
afterEachas-is; it will simply delete the directory created bytmp.
This preserves test semantics while eliminating insecure temporary directory creation.
| @@ -1,7 +1,7 @@ | ||
| import { describe, it, expect, beforeEach, afterEach } from 'vitest'; | ||
| import { writeFileSync, mkdirSync, rmSync } from 'node:fs'; | ||
| import { join } from 'node:path'; | ||
| import { tmpdir } from 'node:os'; | ||
| import tmp from 'tmp'; | ||
| import { loadConfig, getTimeoutFromConfig } from './config.js'; | ||
| import type { TsuConfig } from '../types/config.js'; | ||
|
|
||
| @@ -9,9 +9,8 @@ | ||
| let testDir: string; | ||
|
|
||
| beforeEach(() => { | ||
| // Create a unique temp directory for each test | ||
| testDir = join(tmpdir(), `tsu-test-${Date.now()}-${Math.random().toString(36).slice(2)}`); | ||
| mkdirSync(testDir, { recursive: true }); | ||
| // Create a unique secure temp directory for each test | ||
| testDir = tmp.dirSync({ unsafeCleanup: true }).name; | ||
| }); | ||
|
|
||
| afterEach(() => { |
| @@ -35,7 +35,8 @@ | ||
| "license": "MIT", | ||
| "dependencies": { | ||
| "commander": "^14.0.2", | ||
| "listr2": "^9.0.5" | ||
| "listr2": "^9.0.5", | ||
| "tmp": "^0.2.5" | ||
| }, | ||
| "engines": { | ||
| "node": ">=22.14.0" |
| Package | Version | Security advisories |
| tmp (npm) | 0.2.5 | None |
| it('should prefer .tsurc over other config files', () => { | ||
| const configData1: TsuConfig = { timeout: 5000 }; | ||
| const configData2: TsuConfig = { timeout: 10000 }; | ||
| writeFileSync(join(testDir, '.tsurc'), JSON.stringify(configData1)); |
Check failure
Code scanning / CodeQL
Insecure temporary file High
Copilot Autofix
AI about 2 months ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
| const configData1: TsuConfig = { timeout: 5000 }; | ||
| const configData2: TsuConfig = { timeout: 10000 }; | ||
| writeFileSync(join(testDir, '.tsurc'), JSON.stringify(configData1)); | ||
| writeFileSync(join(testDir, 'tsu.config.json'), JSON.stringify(configData2)); |
Check failure
Code scanning / CodeQL
Insecure temporary file High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
In general, the fix is to avoid manually constructing paths under os.tmpdir() using timestamps or randomness, and instead rely on a library that creates secure, unique, non-raceable temporary files or directories with restrictive permissions (such as the tmp package). This ensures that the directory or file does not already exist and that it is not inadvertently exposed to other users.
For this specific file, the only insecure aspect is the manual creation of testDir using join(tmpdir(), ...) in the beforeEach hook. The best fix is to replace this with a securely created temporary directory via tmp.dirSync(), and then use that directory path for the tests. Concretely:
- Add an import for
tmpat the top ofsrc/utils/config.test.ts. - Change the
beforeEachhook so that instead of computingtestDirwithjoin(tmpdir(), ...)and callingmkdirSync, it callstmp.dirSync({ unsafeCleanup: true }), stores the returnedname(directory path) intestDir, and also stores theTmpDirobject so that it can be cleaned up. - Update
afterEachto usetmpDirObj.removeCallback()instead of manually callingrmSync. This aligns with howtmpmanages its resources and avoids interacting withtmpdir()directly.
These changes leave the rest of the tests unchanged: they continue to write the same files under testDir and call loadConfig(testDir) as before. Only the way testDir is created and cleaned up is modified.
| @@ -1,23 +1,25 @@ | ||
| import { describe, it, expect, beforeEach, afterEach } from 'vitest'; | ||
| import { writeFileSync, mkdirSync, rmSync } from 'node:fs'; | ||
| import { writeFileSync } from 'node:fs'; | ||
| import { join } from 'node:path'; | ||
| import { tmpdir } from 'node:os'; | ||
| import tmp from 'tmp'; | ||
| import { loadConfig, getTimeoutFromConfig } from './config.js'; | ||
| import type { TsuConfig } from '../types/config.js'; | ||
|
|
||
| describe('config', () => { | ||
| let testDir: string; | ||
| let tmpDirObj: tmp.DirResult | null; | ||
|
|
||
| beforeEach(() => { | ||
| // Create a unique temp directory for each test | ||
| testDir = join(tmpdir(), `tsu-test-${Date.now()}-${Math.random().toString(36).slice(2)}`); | ||
| mkdirSync(testDir, { recursive: true }); | ||
| // Create a secure unique temp directory for each test | ||
| tmpDirObj = tmp.dirSync({ unsafeCleanup: true }); | ||
| testDir = tmpDirObj.name; | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| // Clean up temp directory | ||
| if (testDir) { | ||
| rmSync(testDir, { recursive: true, force: true }); | ||
| if (tmpDirObj) { | ||
| tmpDirObj.removeCallback(); | ||
| tmpDirObj = null; | ||
| } | ||
| }); | ||
|
|
| @@ -35,7 +35,8 @@ | ||
| "license": "MIT", | ||
| "dependencies": { | ||
| "commander": "^14.0.2", | ||
| "listr2": "^9.0.5" | ||
| "listr2": "^9.0.5", | ||
| "tmp": "^0.2.5" | ||
| }, | ||
| "engines": { | ||
| "node": ">=22.14.0" |
| Package | Version | Security advisories |
| tmp (npm) | 0.2.5 | None |
| mkdirSync(subDir, { recursive: true }); | ||
|
|
||
| const configData: TsuConfig = { timeout: 7000 }; | ||
| writeFileSync(join(testDir, '.tsurc'), JSON.stringify(configData)); |
Check failure
Code scanning / CodeQL
Insecure temporary file High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
In general, to fix insecure temporary file or directory creation, avoid manually concatenating names under os.tmpdir() and instead use a dedicated temp library or secure API that (a) creates the directory atomically, (b) uses strong randomness for the path, and (c) sets restrictive permissions. For Node.js, the recommended approach is to use the well-tested tmp package.
For this specific code in src/utils/config.test.ts, the main issue is the manual creation of testDir using join(tmpdir(), ...) and mkdirSync. We can replace this with tmp.dirSync() from the tmp package. tmp.dirSync() will create a secure, unique temporary directory and return its path. The rest of the tests can continue to use testDir exactly as before, so the behavior of loadConfig(testDir) and the subsequent file writes is unchanged.
Concretely:
- Add an import for the
tmppackage at the top ofsrc/utils/config.test.ts(e.g.,import tmp from 'tmp';). - In the
beforeEachblock, replace the manualtmpdir()/join/mkdirSynclogic with a single call:testDir = tmp.dirSync({ unsafeCleanup: true }).name;. TheunsafeCleanupoption ensures even non-empty directories can be removed, which matches the existingrmSync(..., { recursive: true, force: true })behavior. - Leave the
afterEachcleanup usingrmSyncas-is; it will still remove thetestDirdirectory tree given the new path. - No other changes to the test logic are necessary.
| @@ -2,6 +2,7 @@ | ||
| import { writeFileSync, mkdirSync, rmSync } from 'node:fs'; | ||
| import { join } from 'node:path'; | ||
| import { tmpdir } from 'node:os'; | ||
| import tmp from 'tmp'; | ||
| import { loadConfig, getTimeoutFromConfig } from './config.js'; | ||
| import type { TsuConfig } from '../types/config.js'; | ||
|
|
||
| @@ -9,9 +10,8 @@ | ||
| let testDir: string; | ||
|
|
||
| beforeEach(() => { | ||
| // Create a unique temp directory for each test | ||
| testDir = join(tmpdir(), `tsu-test-${Date.now()}-${Math.random().toString(36).slice(2)}`); | ||
| mkdirSync(testDir, { recursive: true }); | ||
| // Create a secure unique temp directory for each test | ||
| testDir = tmp.dirSync({ unsafeCleanup: true }).name; | ||
| }); | ||
|
|
||
| afterEach(() => { |
| @@ -35,7 +35,8 @@ | ||
| "license": "MIT", | ||
| "dependencies": { | ||
| "commander": "^14.0.2", | ||
| "listr2": "^9.0.5" | ||
| "listr2": "^9.0.5", | ||
| "tmp": "^0.2.5" | ||
| }, | ||
| "engines": { | ||
| "node": ">=22.14.0" |
| Package | Version | Security advisories |
| tmp (npm) | 0.2.5 | None |
| }, | ||
| }, | ||
| }; | ||
| writeFileSync(join(testDir, '.tsurc'), JSON.stringify(configData)); |
Check failure
Code scanning / CodeQL
Insecure temporary file High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
In general, to fix insecure temporary file or directory creation, avoid manually building paths under os.tmpdir() with timestamps/random strings and then creating them with mkdir/open. Instead, use a library or API that atomically creates a unique temp file/dir with secure permissions, such as the tmp package’s dirSync/fileSync.
For this specific file, we should replace the manual construction of testDir in beforeEach with a call to tmp.dirSync() so that a secure, unique temporary directory is created. Since the rest of the tests treat testDir as “a directory path I can write into”, nothing else needs to change; we just need to store tmpDir.name in testDir and remember to clean it up in afterEach. To avoid extra complexity, we can preserve the current cleanup logic (rmSync(testDir, { recursive: true, force: true })) rather than using tmp’s own automatic cleanup, so existing behavior is unchanged.
Concretely:
- Add an import for
tmpat the top ofsrc/utils/config.test.ts. - Change the
beforeEachblock:- Instead of
testDir = join(tmpdir(), ...)andmkdirSync(testDir, { recursive: true }), calltmp.dirSync({ unsafeCleanup: true })(or without options) and settestDirto the returnednameproperty.
- Instead of
- Leave the rest of the tests as-is; they continue to
join(testDir, ...)and to remove the directory inafterEach.
| @@ -2,6 +2,7 @@ | ||
| import { writeFileSync, mkdirSync, rmSync } from 'node:fs'; | ||
| import { join } from 'node:path'; | ||
| import { tmpdir } from 'node:os'; | ||
| import tmp from 'tmp'; | ||
| import { loadConfig, getTimeoutFromConfig } from './config.js'; | ||
| import type { TsuConfig } from '../types/config.js'; | ||
|
|
||
| @@ -9,9 +10,9 @@ | ||
| let testDir: string; | ||
|
|
||
| beforeEach(() => { | ||
| // Create a unique temp directory for each test | ||
| testDir = join(tmpdir(), `tsu-test-${Date.now()}-${Math.random().toString(36).slice(2)}`); | ||
| mkdirSync(testDir, { recursive: true }); | ||
| // Create a secure unique temp directory for each test | ||
| const tmpDir = tmp.dirSync({ unsafeCleanup: true }); | ||
| testDir = tmpDir.name; | ||
| }); | ||
|
|
||
| afterEach(() => { |
| @@ -35,7 +35,8 @@ | ||
| "license": "MIT", | ||
| "dependencies": { | ||
| "commander": "^14.0.2", | ||
| "listr2": "^9.0.5" | ||
| "listr2": "^9.0.5", | ||
| "tmp": "^0.2.5" | ||
| }, | ||
| "engines": { | ||
| "node": ">=22.14.0" |
| Package | Version | Security advisories |
| tmp (npm) | 0.2.5 | None |
| }); | ||
|
|
||
| it('should throw error for invalid JSON', () => { | ||
| writeFileSync(join(testDir, '.tsurc'), 'invalid json {'); |
Check failure
Code scanning / CodeQL
Insecure temporary file High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
General fix: instead of manually creating a temp directory under os.tmpdir() with join(tmpdir(), ...) and mkdirSync, use a secure temp-directory creation API that creates a unique directory with safe permissions, such as fs.mkdtempSync. This both avoids race conditions and ensures files aren’t created in a shared, predictable path.
Best concrete fix here: change the beforeEach setup so that testDir is created via mkdtempSync with a prefix like join(tmpdir(), 'tsu-test-'). This preserves all existing semantics (a fresh, unique directory per test; same cleanup logic) but uses the built-in secure temp directory primitive. Because the code currently imports mkdirSync but not mkdtempSync, we should update the import from node:fs to include mkdtempSync and stop using mkdirSync in beforeEach. The rest of the test file can remain unchanged.
Specific changes (all in src/utils/config.test.ts):
- Update the import on line 2 to also import
mkdtempSync(you can leavemkdirSyncimported because it’s used later at line 66). - Replace the body of
beforeEach(lines 11–15) so that:testDiris assigned viamkdtempSync(join(tmpdir(), 'tsu-test-')).- The explicit
mkdirSynccall is removed, becausemkdtempSyncalready creates the directory.
No other methods or definitions are required; all uses of testDir already assume it’s a directory path, which remains true.
| @@ -1,5 +1,5 @@ | ||
| import { describe, it, expect, beforeEach, afterEach } from 'vitest'; | ||
| import { writeFileSync, mkdirSync, rmSync } from 'node:fs'; | ||
| import { writeFileSync, mkdirSync, mkdtempSync, rmSync } from 'node:fs'; | ||
| import { join } from 'node:path'; | ||
| import { tmpdir } from 'node:os'; | ||
| import { loadConfig, getTimeoutFromConfig } from './config.js'; | ||
| @@ -9,9 +9,8 @@ | ||
| let testDir: string; | ||
|
|
||
| beforeEach(() => { | ||
| // Create a unique temp directory for each test | ||
| testDir = join(tmpdir(), `tsu-test-${Date.now()}-${Math.random().toString(36).slice(2)}`); | ||
| mkdirSync(testDir, { recursive: true }); | ||
| // Create a unique secure temp directory for each test | ||
| testDir = mkdtempSync(join(tmpdir(), 'tsu-test-')); | ||
| }); | ||
|
|
||
| afterEach(() => { |
Enables users to configure timeout parameters globally or per-check via config file instead of CLI flags.
Changes
Config file loading
.tsurc,.tsurc.json,tsu.config.json,.tsu.config.jsonin project → parent dirs → home directory--with-configflag (no automatic loading)Timeout resolution hierarchy
checks.dart-format.timeout→hook.collate.timeout→timeoutImplementation
src/types/config.ts: Type definitions for config structuresrc/utils/config.ts: Config discovery and timeout resolutionsrc/commands/hook/collate.ts: Timeout application to execFileAsync callsUsage
{ "timeout": 5000, "hook": { "collate": { "timeout": 20000, "checks": { "dart-format": { "timeout": 3000 }, "dart-analysis": { "timeout": 15000 } } } } }Example config template in
templates/.tsurc.example.Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.