fix: add CI workflow, tests, linting, and security hardening by blackboxprogramming · Pull Request #1 · BlackRoad-Archive/road-deploy

blackboxprogramming · 2026-03-10T10:53:10Z

Add GitHub Actions CI workflow (lint + test on Node 18/20/22)
Add ESLint with flat config for code quality checks
Add Jest test suite with 11 tests covering health, deploy validation,
status, webhook, and shellEscape
Fix command injection vulnerabilities by sanitizing all user inputs
before shell execution (shellEscape + domain/path sanitization)
Add build command allowlist to prevent arbitrary command execution
Fix hardcoded port in webhook handler to use PORT env var
Add .gitignore for node_modules, .env, logs
Export app for testability without starting server

https://claude.ai/code/session_014i155SpJut64A5b3NpmBDL

- Add GitHub Actions CI workflow (lint + test on Node 18/20/22) - Add ESLint with flat config for code quality checks - Add Jest test suite with 11 tests covering health, deploy validation, status, webhook, and shellEscape - Fix command injection vulnerabilities by sanitizing all user inputs before shell execution (shellEscape + domain/path sanitization) - Add build command allowlist to prevent arbitrary command execution - Fix hardcoded port in webhook handler to use PORT env var - Add .gitignore for node_modules, .env, logs - Export app for testability without starting server https://claude.ai/code/session_014i155SpJut64A5b3NpmBDL

Copilot

Pull request overview

This PR introduces a baseline CI/testing/linting setup for the Node-based deployment service and attempts to harden shell-executed operations against command injection.

Changes:

Add GitHub Actions workflow to run ESLint + Jest on Node 18/20/22.
Add ESLint flat config and a Jest + Supertest test suite.
Harden deployment commands (shell escaping, domain/path sanitization, build-command allowlist) and export the Express app for testability.

Reviewed changes

Copilot reviewed 5 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`server.js`	Adds shell escaping/sanitization, build command allowlist, PORT usage in webhook, and exports `app` for tests.
`package.json`	Adds `lint`/`test` scripts and dev dependencies for ESLint/Jest/Supertest.
`eslint.config.js`	Introduces ESLint flat config with recommended rules and custom globals/rules.
`__tests__/server.test.js`	Adds Supertest/Jest coverage for health, validation, webhook, status, and `shellEscape`.
`.gitignore`	Ignores Node/build artifacts and local env/log files.
`.github/workflows/ci.yml`	Adds CI job running lint + test across multiple Node versions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-10T10:57:49Z

+        describe: 'readonly',
+        it: 'readonly',
+        test: 'readonly',
+        expect: 'readonly',
+        beforeAll: 'readonly',
+        afterAll: 'readonly',
+        beforeEach: 'readonly',
+        afterEach: 'readonly',
+      },


The Jest globals (describe, it, expect, etc.) are configured as globals for all files, which reduces lint effectiveness in non-test code (production files can accidentally reference test globals without a lint error). Scope these globals to test files only (e.g., a config entry with files: ['**/__tests__/**', '**/*.test.js']) or use the Jest ESLint plugin/env for those patterns.

Copilot · 2026-03-10T10:57:49Z

    "start": "node server.js",
-    "dev": "nodemon server.js"
+    "dev": "nodemon server.js",
+    "test": "jest --forceExit",


Using jest --forceExit can mask real test issues (open handles/timers) and make CI less reliable by forcing a green run even when resources leak. Prefer removing --forceExit and fixing/closing handles in tests; if you need help diagnosing, use --detectOpenHandles (temporarily) or ensure the server/webhook timers are properly mocked/cleared.

Suggested change

"test": "jest --forceExit",

"test": "jest",

Copilot · 2026-03-10T10:57:50Z


+// Sanitize shell arguments to prevent command injection
+function shellEscape(str) {
+  if (typeof str !== 'string') return '';


shellEscape returns an empty string for non-string inputs, which can silently turn invalid request bodies into malformed shell commands (e.g., missing repo_url/branch args) and yield confusing errors. Consider failing fast (throw/return 400 via input validation) when these fields aren’t strings instead of substituting ''.

Suggested change

if (typeof str !== 'string') return '';

if (typeof str !== 'string') {

throw new TypeError('shellEscape expected a string argument');

}

Copilot · 2026-03-10T10:57:50Z

+    const safePath = deploy_path.replace(/[^a-zA-Z0-9._/-]/g, '');
+    const sourceDir = path.join(workdir, safePath);


deploy_path sanitization still allows path traversal (e.g., ../..) and absolute paths. path.join(workdir, safePath) can resolve outside workdir, causing rsync to read/delete unintended locations. Validate deploy_path is a relative subpath (reject .. segments / leading /), and/or after path.resolve(workdir, safePath) ensure it stays within workdir before using it.

Suggested change

const safePath = deploy_path.replace(/[^a-zA-Z0-9._/-]/g, '');

const sourceDir = path.join(workdir, safePath);

const safePath = (deploy_path || '').replace(/[^a-zA-Z0-9._/-]/g, '');

const sourceDir = path.resolve(workdir, safePath);

const normalizedWorkdir = path.resolve(workdir);

const normalizedWorkdirWithSep = normalizedWorkdir.endsWith(path.sep)

? normalizedWorkdir

: normalizedWorkdir + path.sep;

if (sourceDir !== normalizedWorkdir && !sourceDir.startsWith(normalizedWorkdirWithSep)) {

throw new Error(`Invalid deploy_path outside of working directory: ${deploy_path}`);

}

Copilot · 2026-03-10T10:57:50Z

+    const safeDomain = domain.replace(/[^a-zA-Z0-9._-]/g, '');
+    const deployDir = `/home/pi/static-sites/${safeDomain}`;



safeDomain can become empty or differ from the user-supplied domain (e.g., stripping characters), which can lead to deploying into /home/pi/static-sites/ and producing nginx/SSL config for a different host than the API response (url: https://${domain}). Consider validating domain against a strict hostname/FQDN regex and rejecting invalid values (400), then use the validated/sanitized value consistently for deployDir, nginx/certbot, and the returned URL.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c5417edc01

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-10T10:59:18Z

+
+    strategy:
+      matrix:
+        node-version: [18, 20, 22]


Drop unsupported Node 18 from lint/test matrix

The new CI matrix runs npm run lint on Node 18, but this commit also upgrades to eslint@10.0.3, whose lockfile entry requires node: ^20.19.0 || ^22.13.0 || >=24 (package-lock.json), so the Node 18 leg will fail consistently and keep the workflow red for pushes/PRs targeting main/master. Either remove Node 18 from the matrix or pin ESLint to a Node-18-compatible major.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-03-10T10:59:19Z

+    const safeDomain = domain.replace(/[^a-zA-Z0-9._-]/g, '');
+    const deployDir = `/home/pi/static-sites/${safeDomain}`;


Validate sanitized domain before building deploy path

Stripping characters with domain.replace(/[^a-zA-Z0-9._-]/g, '') can produce an empty string (for example, domain: "!!!"), which makes deployDir become /home/pi/static-sites/; the subsequent rsync --delete then targets the shared base directory and can delete other deployed sites. This path should be rejected if sanitization changes it to empty (or otherwise invalid) instead of silently continuing.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-03-10T10:59:19Z

+    const safePath = deploy_path.replace(/[^a-zA-Z0-9._/-]/g, '');
+    const sourceDir = path.join(workdir, safePath);


Reject deploy_path values that sanitize to empty

The new sanitization can collapse deploy_path to '' (e.g., input "$$$"), and path.join(workdir, safePath) then resolves to the repo root, so rsync deploys the entire checkout instead of failing on an invalid build output directory. That silently changes behavior and may publish source files or wrong artifacts; the handler should validate safePath and return an error when it becomes empty.

Useful? React with 👍 / 👎.

Copilot AI review requested due to automatic review settings March 10, 2026 10:53

Copilot started reviewing on behalf of blackboxprogramming March 10, 2026 10:53 View session

Copilot AI reviewed Mar 10, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Mar 10, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: add CI workflow, tests, linting, and security hardening#1

fix: add CI workflow, tests, linting, and security hardening#1
blackboxprogramming wants to merge 1 commit into
mainfrom
claude/fix-repo-workflows-uvmFQ

blackboxprogramming commented Mar 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Mar 10, 2026

Uh oh!

chatgpt-codex-connector Bot Mar 10, 2026

Uh oh!

chatgpt-codex-connector Bot Mar 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-  if (typeof str !== 'string') return '';
+  if (typeof str !== 'string') {
+    throw new TypeError('shellEscape expected a string argument');
+  }

		const safePath = deploy_path.replace(/[^a-zA-Z0-9._/-]/g, '');
		const sourceDir = path.join(workdir, safePath);

-    const safePath = deploy_path.replace(/[^a-zA-Z0-9._/-]/g, '');
-    const sourceDir = path.join(workdir, safePath);
+    const safePath = (deploy_path || '').replace(/[^a-zA-Z0-9._/-]/g, '');
+    const sourceDir = path.resolve(workdir, safePath);
+    const normalizedWorkdir = path.resolve(workdir);
+    const normalizedWorkdirWithSep = normalizedWorkdir.endsWith(path.sep)
+      ? normalizedWorkdir
+      : normalizedWorkdir + path.sep;
+    if (sourceDir !== normalizedWorkdir && !sourceDir.startsWith(normalizedWorkdirWithSep)) {
+      throw new Error(`Invalid deploy_path outside of working directory: ${deploy_path}`);
+    }

		const safeDomain = domain.replace(/[^a-zA-Z0-9._-]/g, '');
		const deployDir = `/home/pi/static-sites/${safeDomain}`;

Uh oh!

Conversation

blackboxprogramming commented Mar 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants