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

Show file tree
Hide file tree
Changes from all commits
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
33 changes: 31 additions & 2 deletions scripts/hourly-commercial-readiness.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,47 @@ function bound(value, limit = MAX_REPORT_DETAIL_CHARS) {
return text.length <= limit ? text : `${text.slice(0, limit)}…`;
}

export function redactSensitiveValue(value, sensitiveValues = []) {
let redacted = String(value ?? "");
for (const sensitiveValue of Array.isArray(sensitiveValues) ? sensitiveValues : []) {
if (typeof sensitiveValue !== "string" || sensitiveValue.length === 0) {
continue;
}
redacted = redacted.split(sensitiveValue).join("[REDACTED]");
}
return redacted;
}

export function createGhSubprocessEnvironment(sourceEnvironment = process.env) {
const childEnvironment = {
GH_HOST: "github.com",
NO_COLOR: "1",
};
if (typeof sourceEnvironment.PATH === "string" && sourceEnvironment.PATH.length > 0) {
childEnvironment.PATH = sourceEnvironment.PATH;
}
if (typeof sourceEnvironment.GH_TOKEN === "string" && sourceEnvironment.GH_TOKEN.length > 0) {
childEnvironment.GH_TOKEN = sourceEnvironment.GH_TOKEN;
}
return childEnvironment;
}
Comment on lines +43 to +55

@coderabbitai coderabbitai Bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

GH_TOKEN을 process.env에서 읽지 마십시오.

Line 32의 기본 인자와 Line 40의 접근은 process.env.GH_TOKEN을 직접 읽습니다. GH_TOKEN은 실제 자격 증명입니다. KV 또는 credential registry에서 토큰을 읽고, 명시적 입력으로 createGhSubprocessEnvironment에 전달하십시오. process.env를 기본 시크릿 소스로 사용하지 마십시오.

As per coding guidelines, scripts/*.mjs: Scripts may read process.env only for non-secret build-time configuration such as file paths and thresholds; real secrets must come from KV or a credential registry.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/hourly-commercial-readiness.mjs` around lines 32 - 44, Update
createGhSubprocessEnvironment so GH_TOKEN is supplied through an explicit
argument populated by the KV or credential registry, rather than read from
sourceEnvironment or process.env. Keep process.env usage limited to non-secret
configuration such as PATH, and include the provided token in the child
environment only when valid.

Source: Coding guidelines


function runGh(args, { input } = {}) {
const childEnvironment = createGhSubprocessEnvironment();
const completed = spawnSync("gh", args, {
encoding: "utf8",
env: childEnvironment,
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)}`);
const detail = redactSensitiveValue(completed.error.message, [childEnvironment.GH_TOKEN]);
throw new Error(`GitHub CLI could not start: ${bound(detail, MAX_ERROR_CHARS)}`);
}
if (completed.status !== 0) {
const detail = completed.stderr || completed.stdout || `exit ${completed.status}`;
const rawDetail = completed.stderr || completed.stdout || `exit ${completed.status}`;
const detail = redactSensitiveValue(rawDetail, [childEnvironment.GH_TOKEN]);
throw new Error(`GitHub CLI failed: ${bound(detail, MAX_ERROR_CHARS)}`);
}
return completed.stdout.trim();
Expand Down
54 changes: 52 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,11 +1,13 @@
import { readFileSync } from "node:fs";
import { describe, expect, it } from "vitest";
import {
createGhSubprocessEnvironment,
flattenArrayPages,
hasActiveNoemaReviewRun,
latestCheckRunsBySuite,
latestReviewStates,
parseNoemaReviewDecision,
redactSensitiveValue,
} from "../scripts/hourly-commercial-readiness.mjs";

const repository = "ContextualWisdomLab/noema";
Expand Down Expand Up @@ -242,11 +244,59 @@ describe("hourly commercial-readiness GitHub adapter", () => {
).toBe(false);
});

it("passes only explicit GitHub CLI authority into child processes", () => {
expect(createGhSubprocessEnvironment({
PATH: "/trusted/bin",
GH_TOKEN: "read-only-maintainer-token",
GH_HOST: "evil.example",
NO_COLOR: "0",
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("redacts an explicit maintainer token before child diagnostics can reach retained outputs", () => {
const token = "read-only-maintainer-token";
const detail = `gh failed with ${token}; retry also exposed ${token}`;

expect(redactSensitiveValue(detail, [token])).toBe(
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed
"gh failed with [REDACTED]; retry also exposed [REDACTED]",
);
expect(redactSensitiveValue(detail, ["", null, undefined, token])).not.toContain(token);
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed
expect(redactSensitiveValue("safe diagnostic", [])).toBe("safe diagnostic");
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed
});

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: childEnvironment");
expect(script).not.toContain("env: process.env");
expect(script).toContain(
"redactSensitiveValue(completed.error.message, [childEnvironment.GH_TOKEN])",
);
expect(script).toContain("redactSensitiveValue(rawDetail, [childEnvironment.GH_TOKEN])");
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,16 +315,16 @@ describe("hourly commercial-readiness GitHub adapter", () => {
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 ambient credential payloads", () => {
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");
expect(script).not.toContain("GH_TOKEN");
expect(script).not.toContain("GITHUB_TOKEN");
expect(script).not.toContain("read-only-maintainer-token");
});

it("documents the operator contract and buyer-visible governance boundaries", () => {
Expand Down
Loading