feat: add github actions ci and core package tests#208
Conversation
- Updated @anthropic-ai/mcpb from 1.2.0 to 2.1.2 (MAJOR) - Updated eslint from 9.39.2 to 10.0.3 (MAJOR) - Updated @eslint/js from 9.39.2 to 10.0.1 (MAJOR) - Updated globals from 15.15.0 to 17.4.0 (MAJOR) - Updated typescript from 5.3.0 to 5.8.4 - Fixed flatted DoS vulnerability (CVE high) Improves: security, MCP compatibility, linting rules, TypeScript features
- Add comprehensive GitHub Actions CI workflow - Multi-node testing (18.x, 20.x, 22.x) - Lint, format, type-check, and test validation - Dependency audit and security checks - Build verification for all packages - Add unit tests for core package configuration loader - Test loadPipelineConfig, loadRalphConfig, loadValidationConfig - Test discoverPipelineConfig functionality - Test default configuration generators - Comprehensive error handling validation - Update critical dependencies to latest stable versions - @vitest/coverage-v8: 4.0.18 → 4.1.0 - lint-staged: 16.2.7 → 16.4.0 - typescript-eslint: 8.55.0 → 8.57.1 - Add CHANGELOG.md for better release tracking Addresses high priority items identified in code review: - Missing CI/CD pipeline for automated quality assurance - No test coverage for critical core package functionality - Outdated dependencies with available security updates
| }, | ||
| }; | ||
|
|
||
| writeFileSync(configPath, JSON.stringify(validConfig, null, 2)); |
Check failure
Code scanning / CodeQL
Insecure temporary file High test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
In general, to fix insecure temporary file usage, avoid manually constructing paths under os.tmpdir() and instead use a well-tested library (such as tmp) that provides race-free, securely permissioned temporary directories/files. These libraries atomically create a unique directory or file with appropriate permissions and return its path, eliminating the need to generate names yourself based on timestamps or PIDs.
For this specific test file, the best fix is to change setupTestDir so it uses tmp to create a secure temporary directory instead of join(tmpdir(), ...) and mkdirSync. We can import tmp at the top of the file, configure it to automatically remove created directories on process exit, and then have setupTestDir call tmp.dirSync({ unsafeCleanup: true }). That call returns an object with a unique directory name that is created securely; we store that in testDir and return it. The rest of the test logic (creating pipeline.json within that directory and cleaning it up with cleanupTestDir) can remain unchanged, because testDir is still just a path to a directory. No behavior visible to the tests changes, other than using a more robust directory creation mechanism.
Concretely:
- In
core/src/__tests__/config-loader.test.js, addimport tmp from 'tmp';alongside the other imports. - Immediately after importing
tmp, calltmp.setGracefulCleanup();so that temporary directories are removed even if tests exit unexpectedly (this is a safe enhancement). - Replace the body of
setupTestDirto create the directory withtmp.dirSync({ unsafeCleanup: true })and assign its.nametotestDirinstead of building it withtmpdir()andmkdirSync. - Leave
cleanupTestDiras-is; it will still removetestDirif it exists. (This is slightly redundant withunsafeCleanup/setGracefulCleanup, but harmless and preserves existing semantics.)
| @@ -7,6 +7,7 @@ | ||
| import { writeFileSync, mkdirSync, rmSync } from 'node:fs'; | ||
| import { join } from 'node:path'; | ||
| import { tmpdir } from 'node:os'; | ||
| import tmp from 'tmp'; | ||
|
|
||
| import { | ||
| loadPipelineConfig, | ||
| @@ -18,13 +19,15 @@ | ||
| getDefaultValidationConfig, | ||
| } from '../config/loader.js'; | ||
|
|
||
| tmp.setGracefulCleanup(); | ||
|
|
||
| describe('Configuration Loader', () => { | ||
| let testDir; | ||
|
|
||
| // Setup test directory before each test | ||
| function setupTestDir() { | ||
| testDir = join(tmpdir(), `appfactory-test-${Date.now()}`); | ||
| mkdirSync(testDir, { recursive: true }); | ||
| const tmpDir = tmp.dirSync({ unsafeCleanup: true }); | ||
| testDir = tmpDir.name; | ||
| return testDir; | ||
| } | ||
|
|
| @@ -44,7 +44,8 @@ | ||
| "dependencies": { | ||
| "chalk": "^5.3.0", | ||
| "ajv": "^8.12.0", | ||
| "zod": "^3.22.0" | ||
| "zod": "^3.22.0", | ||
| "tmp": "^0.2.5" | ||
| }, | ||
| "engines": { | ||
| "node": ">=18.0.0" |
| Package | Version | Security advisories |
| tmp (npm) | 0.2.5 | None |
| const dir = setupTestDir(); | ||
| const configPath = join(dir, 'invalid.json'); | ||
|
|
||
| writeFileSync(configPath, '{ invalid json }'); |
Check failure
Code scanning / CodeQL
Insecure temporary file High test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
General fix: avoid manually constructing paths under os.tmpdir() with predictable names. Instead, use a secure temporary directory/file creation primitive that (a) ensures the path does not already exist and (b) sets appropriate, restrictive permissions. In Node.js, fs.mkdtempSync(prefix) is the standard way to securely create a unique temporary directory under the OS temp directory. For temp files, a library like tmp is appropriate, but here we only need directories.
Best fix here: change setupTestDir so that it uses mkdtempSync with tmpdir() as the parent directory, rather than concatenating tmpdir() with a Date.now()-based name and calling mkdirSync. This keeps functionality identical (a fresh temporary directory for each test run, cleaned up afterwards) while making creation race-free and non-predictable. We can do this by:
- Extending the existing
node:fsimport to includemkdtempSync. - Updating
setupTestDirto compute a prefix viajoin(tmpdir(), 'appfactory-test-')and callmkdtempSyncon it. - Storing the resulting directory path to
testDiras before, socleanupTestDirkeeps working unchanged.
All changes are confined to core/src/__tests__/config-loader.test.js, specifically around the imports and the setupTestDir function (lines 7–29 in the snippet).
| @@ -4,7 +4,7 @@ | ||
|
|
||
| import { test, describe } from 'node:test'; | ||
| import assert from 'node:assert'; | ||
| 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'; | ||
|
|
||
| @@ -23,8 +23,8 @@ | ||
|
|
||
| // Setup test directory before each test | ||
| function setupTestDir() { | ||
| testDir = join(tmpdir(), `appfactory-test-${Date.now()}`); | ||
| mkdirSync(testDir, { recursive: true }); | ||
| const prefix = join(tmpdir(), 'appfactory-test-'); | ||
| testDir = mkdtempSync(prefix); | ||
| return testDir; | ||
| } | ||
|
|
| runE2ETests: true, | ||
| }; | ||
|
|
||
| writeFileSync(configPath, JSON.stringify(validConfig, null, 2)); |
Check failure
Code scanning / CodeQL
Insecure temporary file High test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
To fix this, we should stop constructing a temp directory using tmpdir() plus a predictable suffix. Instead, we should use a library that creates a unique, securely permissioned temporary directory (e.g., tmp.dirSync()) and then use that directory as the test workspace. This removes the predictability and race conditions associated with mkdirSync on a name we compute ourselves.
Concretely, in core/src/__tests__/config-loader.test.js:
- Add an import for the
tmppackage. - Update
setupTestDirso that it callstmp.dirSync({ unsafeCleanup: true })(or justtmp.dirSync()if we don’t want automatic cleanup) and stores the created directory path intestDir. We should keep the existingcleanupTestDirlogic, as it usesrmSyncand matches the rest of the test patterns; we’ll only change howtestDiris created. - Remove the direct use of
tmpdir()and the template stringappfactory-test-${Date.now()}withinsetupTestDir, replacing them with the path returned bytmp.dirSync().
No other test logic needs to change: tests already call setupTestDir() and cleanupTestDir(), so the behavior (using a dedicated temp directory per test run) is preserved while making the underlying directory creation secure.
| @@ -7,6 +7,7 @@ | ||
| import { writeFileSync, mkdirSync, rmSync } from 'node:fs'; | ||
| import { join } from 'node:path'; | ||
| import { tmpdir } from 'node:os'; | ||
| import tmp from 'tmp'; | ||
|
|
||
| import { | ||
| loadPipelineConfig, | ||
| @@ -23,8 +24,8 @@ | ||
|
|
||
| // Setup test directory before each test | ||
| function setupTestDir() { | ||
| testDir = join(tmpdir(), `appfactory-test-${Date.now()}`); | ||
| mkdirSync(testDir, { recursive: true }); | ||
| const tmpDir = tmp.dirSync(); | ||
| testDir = tmpDir.name; | ||
| return testDir; | ||
| } | ||
|
|
| @@ -44,7 +44,8 @@ | ||
| "dependencies": { | ||
| "chalk": "^5.3.0", | ||
| "ajv": "^8.12.0", | ||
| "zod": "^3.22.0" | ||
| "zod": "^3.22.0", | ||
| "tmp": "^0.2.5" | ||
| }, | ||
| "engines": { | ||
| "node": ">=18.0.0" |
| Package | Version | Security advisories |
| tmp (npm) | 0.2.5 | None |
| allowedDotfiles: ['.env.example'], | ||
| }; | ||
|
|
||
| writeFileSync(configPath, JSON.stringify(validConfig, null, 2)); |
Check failure
Code scanning / CodeQL
Insecure temporary file High test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
In general, the way to fix this is to avoid manually constructing paths under os.tmpdir() with predictable names, and instead delegate the creation of temporary files or directories to a library that uses secure, unique names and appropriate permissions. In Node.js, the tmp package is a well‑known option that provides tmp.dirSync() and tmp.fileSync() for this purpose.
For this specific file, the best fix with minimal behavioral change is:
- Import the
tmplibrary alongside the existing imports. - Change
setupTestDir()so that it no longer usestmpdir()andmkdirSync, but instead callstmp.dirSync({ unsafeCleanup: true })(or at leasttmp.dirSync()), stores the returned path intestDir, and returns it. This yields a securely created, unique temporary directory. - Keep
cleanupTestDir()as it is; it simply deletes whatever path was stored intestDir.
All uses of setupTestDir() (including the one leading to the write at line 127) will now operate inside a securely created temp directory, eliminating the insecure creation pattern while preserving the tests’ intent and structure. No other changes inside the tests are required.
| @@ -7,6 +7,7 @@ | ||
| import { writeFileSync, mkdirSync, rmSync } from 'node:fs'; | ||
| import { join } from 'node:path'; | ||
| import { tmpdir } from 'node:os'; | ||
| import tmp from 'tmp'; | ||
|
|
||
| import { | ||
| loadPipelineConfig, | ||
| @@ -23,8 +24,7 @@ | ||
|
|
||
| // Setup test directory before each test | ||
| function setupTestDir() { | ||
| testDir = join(tmpdir(), `appfactory-test-${Date.now()}`); | ||
| mkdirSync(testDir, { recursive: true }); | ||
| testDir = tmp.dirSync({ unsafeCleanup: true }).name; | ||
| return testDir; | ||
| } | ||
|
|
| @@ -44,7 +44,8 @@ | ||
| "dependencies": { | ||
| "chalk": "^5.3.0", | ||
| "ajv": "^8.12.0", | ||
| "zod": "^3.22.0" | ||
| "zod": "^3.22.0", | ||
| "tmp": "^0.2.5" | ||
| }, | ||
| "engines": { | ||
| "node": ">=18.0.0" |
| Package | Version | Security advisories |
| tmp (npm) | 0.2.5 | None |
| }, | ||
| }; | ||
|
|
||
| writeFileSync(configPath, JSON.stringify(validConfig, null, 2)); |
Check failure
Code scanning / CodeQL
Insecure temporary file High test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
In general, insecure temporary file/directory issues are fixed by using a library that atomically creates unique, private temp files or directories (with appropriate permissions) instead of manually building paths under os.tmpdir(). In Node.js, the recommended approach is to use a library like tmp, which provides tmp.dirSync() and tmp.fileSync() that ensure uniqueness and safe permissions.
For this specific code, the main issue is the setupTestDir function, which currently does:
testDir = join(tmpdir(), `appfactory-test-${Date.now()}`);
mkdirSync(testDir, { recursive: true });All uses of testDir (including the one that leads to configPath on line 161) flow from this insecure directory creation. The best fix is to change setupTestDir to delegate directory creation to tmp.dirSync, which returns a unique directory under a secure location, and then use its .name property as testDir. This preserves the external behavior (each test gets a fresh directory, and we still call cleanupTestDir to remove it) while eliminating the race condition and unsafe permissions. We should also add an import for tmp at the top of the file. No other test logic needs to change.
Concretely:
- Add
import tmp from 'tmp';(ESM default import) near the other imports. - Update
setupTestDirso that:- It calls
tmp.dirSync({ unsafeCleanup: true, prefix: 'appfactory-test-' }). - It assigns
testDirto the returned directory’snameproperty. - It returns
testDiras before.
- It calls
unsafeCleanup: true matches the existing behavior of rmSync(testDir, { recursive: true, force: true }), allowing recursive cleanup. All existing calls to setupTestDir and cleanupTestDir remain valid and unchanged.
| @@ -7,6 +7,7 @@ | ||
| import { writeFileSync, mkdirSync, rmSync } from 'node:fs'; | ||
| import { join } from 'node:path'; | ||
| import { tmpdir } from 'node:os'; | ||
| import tmp from 'tmp'; | ||
|
|
||
| import { | ||
| loadPipelineConfig, | ||
| @@ -23,8 +24,8 @@ | ||
|
|
||
| // Setup test directory before each test | ||
| function setupTestDir() { | ||
| testDir = join(tmpdir(), `appfactory-test-${Date.now()}`); | ||
| mkdirSync(testDir, { recursive: true }); | ||
| const tmpDir = tmp.dirSync({ unsafeCleanup: true, prefix: 'appfactory-test-' }); | ||
| testDir = tmpDir.name; | ||
| return testDir; | ||
| } | ||
|
|
| @@ -44,7 +44,8 @@ | ||
| "dependencies": { | ||
| "chalk": "^5.3.0", | ||
| "ajv": "^8.12.0", | ||
| "zod": "^3.22.0" | ||
| "zod": "^3.22.0", | ||
| "tmp": "^0.2.5" | ||
| }, | ||
| "engines": { | ||
| "node": ">=18.0.0" |
| Package | Version | Security advisories |
| tmp (npm) | 0.2.5 | None |
| }, | ||
| }; | ||
|
|
||
| writeFileSync(configPath, JSON.stringify(validConfig, null, 2)); |
Check failure
Code scanning / CodeQL
Insecure temporary file High test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
In general, the issue is fixed by not constructing temp paths manually under os.tmpdir(). Instead, use a library that atomically creates a unique, private temporary file or directory (like the tmp package). This avoids race conditions and prevents other users from pre-creating or accessing the directory/file.
For this specific file, the best fix is to replace the ad‑hoc setupTestDir implementation (which uses tmpdir() and mkdirSync) with a call to tmp.dirSync that returns a securely created temporary directory, and to add the corresponding import. All tests already use setupTestDir() to get their working directory, so changing its implementation preserves existing behavior while making the directory creation secure. The line CodeQL flags (writeFileSync(configPath, ...) at line 189) will then operate on a directory that was safely created by tmp, addressing the root cause.
Concrete changes in core/src/__tests__/config-loader.test.js:
- Add
import tmp from 'tmp';alongside the other imports. - Update
setupTestDir():- Remove
join(tmpdir(), \appfactory-test-${Date.now()}`)andmkdirSync(...)`. - Replace them with
tmp.dirSync({ prefix: 'appfactory-test-', unsafeCleanup: true }).name, which creates a unique temp directory securely and returns its path.
- Remove
- Keep
cleanupTestDir()as-is; it simply removes whatevertestDiris set to.
No other code in this file needs to change.
| @@ -7,6 +7,7 @@ | ||
| import { writeFileSync, mkdirSync, rmSync } from 'node:fs'; | ||
| import { join } from 'node:path'; | ||
| import { tmpdir } from 'node:os'; | ||
| import tmp from 'tmp'; | ||
|
|
||
| import { | ||
| loadPipelineConfig, | ||
| @@ -23,8 +24,9 @@ | ||
|
|
||
| // Setup test directory before each test | ||
| function setupTestDir() { | ||
| testDir = join(tmpdir(), `appfactory-test-${Date.now()}`); | ||
| mkdirSync(testDir, { recursive: true }); | ||
| // Use a securely created temporary directory to avoid race conditions and | ||
| // ensure the directory is not pre-created or accessible by other users. | ||
| testDir = tmp.dirSync({ prefix: 'appfactory-test-', unsafeCleanup: true }).name; | ||
| return testDir; | ||
| } | ||
|
|
| @@ -44,7 +44,8 @@ | ||
| "dependencies": { | ||
| "chalk": "^5.3.0", | ||
| "ajv": "^8.12.0", | ||
| "zod": "^3.22.0" | ||
| "zod": "^3.22.0", | ||
| "tmp": "^0.2.5" | ||
| }, | ||
| "engines": { | ||
| "node": ">=18.0.0" |
| Package | Version | Security advisories |
| tmp (npm) | 0.2.5 | None |
- Run npm audit fix on mcp-server workspace - Fixed 5 high and moderate severity vulnerabilities - Remaining 5 low severity vulnerabilities have no fix available
What's Changed
This PR addresses critical infrastructure and testing gaps identified during the AppFactory code review.
🚀 Added
GitHub Actions CI workflow with comprehensive testing
Unit tests for core package configuration loader
loadPipelineConfig,loadRalphConfig,loadValidationConfigdiscoverPipelineConfigfunctionalityCHANGELOG.md for better release tracking and version history
🔧 Updated Dependencies
🏗️ Benefits
✅ Testing
📋 Addresses Review Findings
This establishes the foundation for reliable automated testing and quality assurance as AppFactory scales.