Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions .github/workflows/deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -901,8 +901,9 @@ jobs:
# Uses the same pattern as verify-stabilization.sh: only relative paths
# (./infra/ or ../infra/) are forbidden. Absolute /opt/infra is allowed.
# Scope: scripts/ and src/ only (not workflows where guard steps live).
if grep -rE "\./infra/|\.\.\./infra/" scripts/ src/ \
--binary-files=without-match --exclude-dir=node_modules 2>/dev/null \
if grep -rE "\./infra/|\.\./infra/" scripts/ src/ \
--binary-files=without-match --exclude-dir=node_modules \
--exclude=verify-stabilization.sh 2>/dev/null \
| grep -Ev '^[^:]+:\s*(#|//)'; then
echo "::error::Local repo-relative infra coupling (./infra/ or ../infra/) detected in scripts/ or src/"
exit 1
Expand Down
10 changes: 10 additions & 0 deletions .github/workflows/pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,12 @@ jobs:

echo "βœ“ /health returned 200"

# Same binary + script path as Docker HEALTHCHECK (exec form); catches ESM/require
# regressions and confirms 127.0.0.1:3000/health from inside the container.
echo "Validating /app/healthcheck.js (ESM, distroless node)..."
docker exec api-ci-test /nodejs/bin/node /app/healthcheck.js
echo "βœ“ healthcheck.js exited 0"

# Smoke tests: admin endpoints must reject unauthenticated requests with 401
for ENDPOINT in /admin/audit-log /admin/webhook-dlq; do
ECODE=$(docker run --rm \
Expand Down Expand Up @@ -541,6 +547,10 @@ jobs:
exit 1
fi

echo "Validating /app/healthcheck.js (same as Docker HEALTHCHECK)..."
docker exec api-blue /nodejs/bin/node /app/healthcheck.js
echo "βœ“ healthcheck.js exited 0"

# Smoke: auth guards must reject unauthenticated requests with 401.
for ENDPOINT in /admin/audit-log /admin/webhook-dlq; do
CODE=$(docker run --rm \
Expand Down
70 changes: 47 additions & 23 deletions healthcheck.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
// Distroless-compatible liveness probe (no curl). Semantics align with:
// curl -fsS http://127.0.0.1:3000/health || exit 1
// ESM: package.json has "type":"module"; this file must not use require().
// - 127.0.0.1 + IPv4 only (avoid ::1 / dual-stack quirks)
// - exit 0 only on HTTP 200; any other status or error β†’ exit 1
// - bounded wall time < Docker --timeout (5s)
'use strict';

const http = require('http');
import http from 'node:http';

const TIMEOUT_MS = 4500;
let settled = false;
Expand All @@ -18,26 +17,51 @@
process.exit(code);
}

const req = http.request(
{
host: '127.0.0.1',
port: 3000,
path: '/health',
method: 'GET',
family: 4,
},
(res) => {
res.on('data', () => {});
res.on('end', () => {
finish(res.statusCode === 200 ? 0 : 1);
});
res.on('error', () => finish(1));
},
);
function logErr(prefix, err) {
console.error(`[healthcheck] ${prefix}`, err);

Check warning

Code scanning / CodeQL

Log injection Medium

Log entry depends on a
user-provided value
.

Copilot Autofix

AI about 2 months ago

To fix this, we should ensure that potentially user-influenced content in err cannot inject new log lines or otherwise confuse log consumers. The general approach is to sanitize any string representation of err before logging: strip \n and \r characters (and optionally other control characters) and ensure the log makes clear where user-controlled content starts.

The best minimal-change fix here is to change logErr so it does not pass the raw err object directly to console.error. Instead, we can derive a safe string representation (using String(err) or err.message and optionally err.stack), remove any newline and carriage-return characters from the message we log on a single line, and then log that single sanitized string. This keeps existing behavior (healthcheck still logs errors, still exits with the same codes) while preventing multi-line log spoofing. No external packages are needed; we can use built-in JS string methods.

Concretely, in healthcheck.js, we will replace the body of logErr (lines 20–22) with logic that:

  • Converts err into a descriptive string (e.g., including name and message, and maybe a truncated/sanitized stack on a separate pass).
  • Removes \r and \n from the parts that might contain attacker input.
  • Logs a single, clearly delimited string via console.error.

We will not touch the rest of the file; all existing calls to logErr remain unchanged.

Suggested changeset 1
healthcheck.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/healthcheck.js b/healthcheck.js
--- a/healthcheck.js
+++ b/healthcheck.js
@@ -18,7 +18,10 @@
 }
 
 function logErr(prefix, err) {
-  console.error(`[healthcheck] ${prefix}`, err);
+  // Sanitize error output to avoid log injection via newlines in error messages
+  const errStr = String(err);
+  const sanitizedErrStr = errStr.replace(/[\r\n]+/g, ' ');
+  console.error(`[healthcheck] ${prefix}: ${sanitizedErrStr}`);
 }
 
 process.on('uncaughtException', (err) => {
EOF
@@ -18,7 +18,10 @@
}

function logErr(prefix, err) {
console.error(`[healthcheck] ${prefix}`, err);
// Sanitize error output to avoid log injection via newlines in error messages
const errStr = String(err);
const sanitizedErrStr = errStr.replace(/[\r\n]+/g, ' ');
console.error(`[healthcheck] ${prefix}: ${sanitizedErrStr}`);
}

process.on('uncaughtException', (err) => {
Copilot is powered by AI and may make mistakes. Always verify output.
}

process.on('uncaughtException', (err) => {
logErr('uncaughtException', err);
finish(1);
});

req.on('error', () => finish(1));
req.setTimeout(TIMEOUT_MS, () => {
req.destroy();
process.on('unhandledRejection', (reason) => {
logErr('unhandledRejection', reason);
finish(1);
});
req.end();

try {
const req = http.request(
{
host: '127.0.0.1',
port: 3000,
path: '/health',
method: 'GET',
family: 4,
},
(res) => {
res.on('data', () => {});
res.on('end', () => {
finish(res.statusCode === 200 ? 0 : 1);
});
res.on('error', (err) => {
logErr('response error', err);
finish(1);
});
},
);

req.on('error', (err) => {
logErr('request error', err);
finish(1);
});
req.setTimeout(TIMEOUT_MS, () => {
req.destroy();
finish(1);
});
req.end();
} catch (err) {
logErr('fatal', err);
finish(1);
}
6 changes: 4 additions & 2 deletions scripts/deploy.sh
Original file line number Diff line number Diff line change
Expand Up @@ -188,11 +188,13 @@ _ft_snapshot() {
}

# ---------------------------------------------------------------------------
# GITHUB ACTIONS SUMMARY
# GITHUB ACTIONS SUMMARY (optional; unset outside GitHub Actions β€” must not trip set -u)
# ---------------------------------------------------------------------------
_ft_github_summary() {
local status="$1" container="${2:-unknown}" image="${3:-unknown}" reason="${4:-}"
[ -z "$GITHUB_STEP_SUMMARY" ] && return 0
if [ -z "${GITHUB_STEP_SUMMARY:-}" ]; then
return 0
fi
{
echo "### πŸš€ Deployment Summary"
echo "| Field | Value |"
Expand Down
Loading