Skip to content

fix(security): isolate hourly readiness gh environment #109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
seonghobae wants to merge 5 commits into from
Closed

fix(security): isolate hourly readiness gh environment #109

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e76d8be
test(security): require least-authority gh subprocess env
seonghobae Aug 10, 2026
05d6a0a
fix(security): isolate hourly gh subprocess authority
seonghobae Aug 10, 2026
55a11db
test(security): require maintainer token diagnostic redaction
seonghobae Aug 10, 2026
1ef50c7
fix(security): redact maintainer token from gh diagnostics
seonghobae Aug 10, 2026
11618bb
test(security): bind gh diagnostic redaction contracts
seonghobae Aug 10, 2026
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
35 changes: 33 additions & 2 deletions test/hourly-commercial-readiness-script.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { readFileSync } from "node:fs";
import { describe, expect, it } from "vitest";
import {
createGhSubprocessEnvironment,
flattenArrayPages,
hasActiveNoemaReviewRun,
latestCheckRunsBySuite,
Expand Down Expand Up @@ -242,11 +243,42 @@
).toBe(false);
});

it("passes only explicit GitHub CLI authority into child processes", () => {
expect(createGhSubprocessEnvironment({

Check failure on line 247 in test/hourly-commercial-readiness-script.test.ts

View workflow job for this annotation

GitHub Actions / verify

test/hourly-commercial-readiness-script.test.ts > hourly commercial-readiness GitHub adapter > passes only explicit GitHub CLI authority into child processes 

TypeError: createGhSubprocessEnvironment is not a function
 ❯ test/hourly-commercial-readiness-script.test.ts:247:12
PATH: "/trusted/bin",
GH_TOKEN: "read-only-maintainer-token",
GITHUB_TOKEN: "ambient-workflow-token",
NVIDIA_NIM_API_KEY: "model-secret",
NOEMA_MAINTAINER_APP_PRIVATE_KEY: "maintainer-private-key",
NOEMA_REVIEWER_APP_PRIVATE_KEY: "reviewer-private-key",
NOEMA_REVIEWER_LOGIN: "reviewer[bot]",
CLOUDFLARE_API_TOKEN: "cloudflare-secret",
HTTPS_PROXY: "http://proxy.invalid",
HTTP_PROXY: "http://proxy.invalid",
ALL_PROXY: "socks5://proxy.invalid",
HOME: "/credential-bearing-home",
NODE_OPTIONS: "--require /tmp/preload.cjs",
NOEMA_MAINTENANCE_ENABLED: "true",
})).toEqual({
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed
GH_HOST: "github.com",
NO_COLOR: "1",
PATH: "/trusted/bin",
GH_TOKEN: "read-only-maintainer-token",
});

expect(createGhSubprocessEnvironment({})).toEqual({
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed
GH_HOST: "github.com",
NO_COLOR: "1",
});
});

it("uses shell-free complete pagination and exact-head write contracts", () => {
const script = readFileSync("scripts/hourly-commercial-readiness.mjs", "utf8");

expect(script).toContain('spawnSync("gh"');
expect(script).toContain("shell: false");
expect(script).toContain("env: createGhSubprocessEnvironment()");

Check failure on line 280 in test/hourly-commercial-readiness-script.test.ts

View workflow job for this annotation

GitHub Actions / verify

test/hourly-commercial-readiness-script.test.ts > hourly commercial-readiness GitHub adapter > uses shell-free complete pagination and exact-head write contracts 

AssertionError: expected '#!/usr/bin/env node\nimport { spawnSy…' to contain 'env: createGhSubprocessEnvironment()'

- Expected
+ Received

- env: createGhSubprocessEnvironment()
+ #!/usr/bin/env node
+ import { spawnSync } from "node:child_process";
+ import { appendFileSync, mkdirSync, writeFileSync } from "node:fs";
+ import { dirname, resolve } from "node:path";
+ import { pathToFileURL } from "node:url";
+ import { evaluatePullRequest } from "./lib/commercial-readiness-loop.mjs";
+
+ const MAX_ERROR_CHARS = 4_000;
+ const MAX_REPORT_DETAIL_CHARS = 1_000;
+ const MAX_GH_OUTPUT_BYTES = 16 * 1024 * 1024;
+ const repositoryPattern = /^ContextualWisdomLab\/[A-Za-z0-9_.-]+$/;
+ const botLoginPattern = /^[A-Za-z0-9-]+\[bot\]$/;
+ const fullShaPattern = /^[0-9a-f]{40}$/i;
+ const noemaMarkerPattern = /<!--\s*noema-review-gate\s+head_sha=([0-9a-f]{40})\s+decision=(approve|request_changes|blocked)\s*-->/gi;
+ const noemaCredentialMarker = "Reviewer credential: `noema-github-app`";
+ const reviewThreadQuery = "query($owner:String!,$name:String!,$number:Int!,$endCursor:String){repository(owner:$owner,name:$name){pullRequest(number:$number){reviewThreads(first:100,after:$endCursor){nodes{isResolved}pageInfo{hasNextPage endCursor}}}}}";
+ const activeWorkflowRunStatuses = new Set([
+   "requested",
+   "waiting",
+   "pending",
+   "queued",
+   "in_progress",
+ ]);
+
+ function bound(value, limit = MAX_REPORT_DETAIL_CHARS) {
+   const text = String(value ?? "")
+     .replace(/[\u0000-\u0008\u000b\u000c\u000e-\u001f\u007f]/g, "")
+     .trim();
+   return text.length <= limit ? text : `${text.slice(0, limit)}…`;
+ }
+
+ function runGh(args, { input } = {}) {
+   const completed = spawnSync("gh", args, {
+     encoding: "utf8",
+     input,
+     maxBuffer: MAX_GH_OUTPUT_BYTES,
+     shell: false,
+   });
+   if (completed.error) {
+     throw new Error(`GitHub CLI could not start: ${bound(completed.error.message, MAX_ERROR_CHARS)}`);
+   }
+   if (completed.status !== 0) {
+     const detail = completed.stderr || completed.stdout || `exit ${completed.status}`;
+     throw new Error(`GitHub CLI failed: ${bound(detail, MAX_ERROR_CHARS)}`);
+   }
+   return completed.stdout.trim();
+ }
+
+ function runGhJson(args, options) {
+   const raw = runGh(args, options);
+   if (!raw) {
+     return null;
+   }
+   try {
+     return JSON.parse(raw);
+   } catch (error) {
+     throw new Error(`GitHub CLI returned invalid JSON: ${bound(error.message, MAX_ERROR_CHARS)}`);
+   }
+ }
+
+ export function flattenArrayPages(pages) {
+   if (!Array.isArray(pages)) {
+     throw new TypeError("Paginated GitHub response must be an array of pages.");
+   }
+   return pages.flatMap((page) => {
+     if (!Array.isArray(page)) {
+       throw new TypeError("Each paginated GitHub array page must be an array.");
+     }
+     return page;
+   });
+ }
+
+ function checkRunTimestamp(check) {
+   return Math.max(
+     Date.parse(check?.completed_at || "") || 0,
+     Date.parse(check?.started_at || "") || 0,
+   );
+ }
+
+ function checkRunChronologicalOrder(left, right) {
+   const leftId = Number(left?.id);
+   const rightId = Number(right?.id);
+   if (
+     Number.isSafeInteger(leftId)
+     && Number.isSafeInteger(rightId)
+     && leftId !== rightId
+   ) {
+     return leftId - rightId;
+   }
+   const timeDelta = checkRunTimestamp(left) - checkRunTimestamp(right);
+   if (timeDelta !== 0) {
+     return timeDelta;
+   }
+   return leftId - rightId;
+ }
+
+ function checkRunSuiteKey(check) {
+   const checkId = Number(check?.id);
+   const suiteId = Number(check?.check_suite?.id);
+   const name = String(check?.name ?? "").trim();
+   const appSlug = String(check?.app?.slug ?? "").trim().toLowerCase();
+   if (
+     !Number.isSafeInteger(checkId)
+     || checkId <= 0
+     || !Number.isSafeInteger(suiteId)
+     || suiteId <= 0
+     || !name
+     || !appSlug
+   ) {
+     return null;
+   }
+   return `${suiteId}\u0000${appSlug}\u0000${name}`;
+ }
+
+ export function latestCheckRunsBySuite(checkRuns) {
+   const latestBySuite = new Map();
+
expect(script).not.toContain("env: process.env");
expect(script).toContain('"--paginate", "--slurp"');
expect(script).toContain("pulls?state=open&per_page=100");
expect(script).toContain("check-runs?filter=all&per_page=100");
Expand All @@ -265,15 +297,14 @@
expect(script).toContain("live?.head?.repo?.full_name !== repository");
});

it("writes a bounded report and post-action queue outputs without exposing tokens", () => {
it("writes a bounded report and post-action queue outputs without embedding ambient token names", () => {
const script = readFileSync("scripts/hourly-commercial-readiness.mjs", "utf8");

expect(script).toContain("open_pull_request_count=");
expect(script).toContain("remaining_open_pull_request_count=");
expect(script).toContain("report_path=");
expect(script).toContain("remainingOpenPullRequestCount");
expect(script).toContain("MAX_ERROR_CHARS");
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
expect(script).not.toContain("GH_TOKEN");
expect(script).not.toContain("GITHUB_TOKEN");
});

Expand Down
Loading