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
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

In general, to fix log injection in plain-text logs, remove or encode line breaks and other control characters from user-controlled data before logging them, and keep log format clearly delineated so injected text cannot masquerade as separate log entries.

For this specific code, the cleanest fix without changing observable functionality is:

  • Add a small helper that converts any error-like value into a safe, single-line string: it will stringify the error (using stack or message or String(err)), then strip \r and \n so the log output remains on one line.
  • Update logErr to log this sanitized string instead of the raw err argument. This preserves the information content (message/stack) while preventing an attacker from injecting extra log lines.

Concretely:

  • In healthcheck.js, define a helper function (e.g., formatErrorForLog) above logErr.
  • Modify logErr(prefix, err) so that instead of console.error('[healthcheck] ${prefix}', err); it builds a single string combining the prefix and a sanitized representation of err, and passes that to console.error. No new imports are required; we can use basic string methods and regex replacement.

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
@@ -17,8 +17,30 @@
   process.exit(code);
 }
 
+function formatErrorForLog(err) {
+  let msg;
+  if (err && typeof err === 'object') {
+    if (typeof err.stack === 'string') {
+      msg = err.stack;
+    } else if (typeof err.message === 'string') {
+      msg = err.message;
+    } else {
+      try {
+        msg = JSON.stringify(err);
+      } catch {
+        msg = String(err);
+      }
+    }
+  } else {
+    msg = String(err);
+  }
+  // Remove newline characters to prevent log injection via line breaks.
+  return msg.replace(/[\r\n]+/g, ' ');
+}
+
 function logErr(prefix, err) {
-  console.error(`[healthcheck] ${prefix}`, err);
+  const safeErr = formatErrorForLog(err);
+  console.error(`[healthcheck] ${prefix}: ${safeErr}`);
 }
 
 process.on('uncaughtException', (err) => {
EOF
@@ -17,8 +17,30 @@
process.exit(code);
}

function formatErrorForLog(err) {
let msg;
if (err && typeof err === 'object') {
if (typeof err.stack === 'string') {
msg = err.stack;
} else if (typeof err.message === 'string') {
msg = err.message;
} else {
try {
msg = JSON.stringify(err);
} catch {
msg = String(err);
}
}
} else {
msg = String(err);
}
// Remove newline characters to prevent log injection via line breaks.
return msg.replace(/[\r\n]+/g, ' ');
}

function logErr(prefix, err) {
console.error(`[healthcheck] ${prefix}`, err);
const safeErr = formatErrorForLog(err);
console.error(`[healthcheck] ${prefix}: ${safeErr}`);
}

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);
}
Loading