diff --git a/.env.selfhost.example b/.env.selfhost.example index 1b5905b0a0..09e3120c42 100644 --- a/.env.selfhost.example +++ b/.env.selfhost.example @@ -16,16 +16,21 @@ GITHUB_APP_ID=123456 GITHUB_APP_SLUG=my-gittensory-app GITHUB_APP_PRIVATE_KEY_FILE=/run/secrets/github-app-private-key.pem # or GITHUB_APP_PRIVATE_KEY= inline -GITHUB_WEBHOOK_SECRET=change-this-long-random-value GITTENSOR_REGISTRY_URL=https://example.invalid/registry.json -GITTENSORY_API_TOKEN=change-this-32-byte-random-token -GITTENSORY_MCP_TOKEN=change-this-32-byte-random-token -INTERNAL_JOB_TOKEN=change-this-32-byte-random-token + +# The four secrets below are commented out ON PURPOSE — do not paste a placeholder string here. +# Generate a real random value for EACH ONE individually (e.g. `openssl rand -hex 32`) and uncomment +# it; the app's boot-time preflight check refuses to start with a known-placeholder or too-short +# value, so copy-pasting the same string into more than one of these will also fail preflight. +# GITHUB_WEBHOOK_SECRET= # the sole HMAC key GitHub webhook deliveries are verified against +# GITTENSORY_API_TOKEN= # server-to-server API bearer token — bypasses per-repo write checks +# GITTENSORY_MCP_TOKEN= # shared MCP bearer token +# INTERNAL_JOB_TOKEN= # gates internal-only routes # REQUIRED once, for the first-run /setup wizard that creates the GitHub App manifest. Not needed # after the App credentials above are filled in. PUBLIC_API_ORIGIN=https://reviews.example.com -SELFHOST_SETUP_TOKEN=change-this-long-random-value +# SELFHOST_SETUP_TOKEN= # generate a real random value the same way as above before setting this # REQUIRED for control-panel access. Comma/whitespace-separated GitHub logins. FAIL-CLOSED: unset # means nobody (not even you) can sign into the dashboard. diff --git a/apps/gittensory-ui/src/lib/selfhost-env-reference.ts b/apps/gittensory-ui/src/lib/selfhost-env-reference.ts index 968db7e60b..5cf550a5db 100644 --- a/apps/gittensory-ui/src/lib/selfhost-env-reference.ts +++ b/apps/gittensory-ui/src/lib/selfhost-env-reference.ts @@ -83,7 +83,7 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ }, { name: "DATABASE_URL", - firstReference: "src/selfhost/preflight.ts:140", + firstReference: "src/selfhost/preflight.ts:201", }, { name: "DISCORD_REPO_WEBHOOKS", @@ -231,7 +231,7 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ }, { name: "PUBLIC_API_ORIGIN", - firstReference: "src/selfhost/preflight.ts:131", + firstReference: "src/selfhost/preflight.ts:192", }, { name: "QDRANT_API_KEY", @@ -251,7 +251,7 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ }, { name: "REDIS_URL", - firstReference: "src/selfhost/preflight.ts:83", + firstReference: "src/selfhost/preflight.ts:144", }, { name: "REVIEW_AUDIT_DIR", @@ -267,7 +267,7 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ }, { name: "SELFHOST_SETUP_TOKEN", - firstReference: "src/selfhost/preflight.ts:125", + firstReference: "src/selfhost/preflight.ts:186", }, { name: "SENTRY_DSN", @@ -317,7 +317,7 @@ export const SELFHOST_ENV_REFERENCE_MARKDOWN = [ "| `CODEX_AI_TIMEOUT_MS` | `src/selfhost/ai.ts:112` |", "| `CRON_INTERVAL_MS` | `src/server.ts:818` |", "| `DATABASE_PATH` | `src/server.ts:239` |", - "| `DATABASE_URL` | `src/selfhost/preflight.ts:140` |", + "| `DATABASE_URL` | `src/selfhost/preflight.ts:201` |", "| `DISCORD_REPO_WEBHOOKS` | `src/selfhost/discord-notify.ts:31` |", "| `DISCORD_WEBHOOK_URL` | `src/selfhost/discord-notify.ts:40` |", "| `GITHUB_APP_ID` | `src/selfhost/orb-collector.ts:59` |", @@ -354,16 +354,16 @@ export const SELFHOST_ENV_REFERENCE_MARKDOWN = [ "| `OTEL_TRACES_SAMPLER_ARG` | `src/selfhost/otel.ts:76` |", "| `PGVECTOR_ENABLED` | `src/server.ts:219` |", "| `PORT` | `src/server.ts:622` |", - "| `PUBLIC_API_ORIGIN` | `src/selfhost/preflight.ts:131` |", + "| `PUBLIC_API_ORIGIN` | `src/selfhost/preflight.ts:192` |", "| `QDRANT_API_KEY` | `src/selfhost/qdrant-vectorize.ts:50` |", "| `QDRANT_DIM` | `src/selfhost/qdrant-vectorize.ts:71` |", "| `QDRANT_URL` | `src/server.ts:503` |", "| `QUEUE_BACKGROUND_CONCURRENCY` | `src/selfhost/queue-common.ts:102` |", - "| `REDIS_URL` | `src/selfhost/preflight.ts:83` |", + "| `REDIS_URL` | `src/selfhost/preflight.ts:144` |", "| `REVIEW_AUDIT_DIR` | `src/server.ts:548` |", "| `SELFHOST_BUNDLE_ALL` | `scripts/build-selfhost.mjs:13` |", "| `SELFHOST_SERVICE` | `scripts/smoke-observability-traces.mjs:5` |", - "| `SELFHOST_SETUP_TOKEN` | `src/selfhost/preflight.ts:125` |", + "| `SELFHOST_SETUP_TOKEN` | `src/selfhost/preflight.ts:186` |", "| `SENTRY_DSN` | `src/selfhost/sentry.ts:355` |", "| `SENTRY_ENVIRONMENT` | `src/selfhost/otel.ts:60` |", "| `SENTRY_RELEASE` | `src/selfhost/otel.ts:62` |", diff --git a/apps/gittensory-ui/src/routes/docs.self-hosting-quickstart.tsx b/apps/gittensory-ui/src/routes/docs.self-hosting-quickstart.tsx index 645f28e288..f500ece297 100644 --- a/apps/gittensory-ui/src/routes/docs.self-hosting-quickstart.tsx +++ b/apps/gittensory-ui/src/routes/docs.self-hosting-quickstart.tsx @@ -44,6 +44,15 @@ function SelfHostingQuickstart() { code={`cp .env.selfhost.example .env # edit .env`} /> + + The webhook secret and static bearer tokens (GITHUB_WEBHOOK_SECRET,{" "} + GITTENSORY_API_TOKEN, GITTENSORY_MCP_TOKEN,{" "} + INTERNAL_JOB_TOKEN, SELFHOST_SETUP_TOKEN) ship commented out on + purpose. Generate a distinct random value for each one (e.g.{" "} + openssl rand -hex 32) — never reuse the same string across more than one of + them. The app refuses to boot if any of these is left at a known-placeholder or too-short + value. + .env.selfhost.example already ships a conservative starting config —{" "} dry-run mode, a small repo allowlist, unified comments, safety, and grounding, diff --git a/src/selfhost/preflight.ts b/src/selfhost/preflight.ts index 7a38769b17..991e55c4dc 100644 --- a/src/selfhost/preflight.ts +++ b/src/selfhost/preflight.ts @@ -77,6 +77,67 @@ function addProblem( problems.push({ var: name, message }); } +// Codex security finding: `.env.selfhost.example` / `.env.example` ship these EXACT literal placeholder +// values for high-privilege secrets (the webhook HMAC secret, plus the static API/MCP/internal bearer +// tokens). An operator who copies the starter to `.env` and misses "fill in the placeholders" runs an +// instance with a PUBLICLY KNOWN webhook secret (forgeable signatures) and PUBLICLY KNOWN bearer tokens +// (GITTENSORY_API_TOKEN bypasses app-role + per-repo write checks; INTERNAL_JOB_TOKEN gates internal +// routes) -- silently, with no error. Reject these exact strings at boot rather than trusting every +// operator to have actually edited the file. +const KNOWN_PLACEHOLDER_SECRETS = new Set([ + "change-this-long-random-value", + "change-this-32-byte-random-token", +]); + +// A generated random secret (openssl rand -hex 32 = 64 chars, or base64 32 bytes ~= 44 chars) is always +// far longer than this; a human-typed guess or a short password essentially never reaches it. Not a +// substitute for the exact-match blocklist above (a placeholder could in principle be long), but catches +// the much broader class of "technically non-blank, not actually a secret." +const MIN_SECRET_LENGTH = 20; + +const CRITICAL_SECRET_VARS = [ + "GITHUB_WEBHOOK_SECRET", + "GITTENSORY_API_TOKEN", + "GITTENSORY_MCP_TOKEN", + "INTERNAL_JOB_TOKEN", + "SELFHOST_SETUP_TOKEN", +] as const; + +/** Validate one critical secret's STRENGTH (never its presence -- callers decide whether a given var is + * required in the current deployment mode). Returns null when the value is fine to use. Never echoes the + * supplied value back in the message: an unsafe secret is exactly the value that must not appear in logs. */ +function criticalSecretProblem(name: string, value: string): string | null { + if (KNOWN_PLACEHOLDER_SECRETS.has(value)) + return `${name} is still set to the placeholder value shipped in .env.selfhost.example / .env.example. Generate a real random secret (e.g. \`openssl rand -hex 32\`) before running this instance.`; + if (value.length < MIN_SECRET_LENGTH) + return `${name} is too short (${value.length} chars, minimum ${MIN_SECRET_LENGTH}) to be a safe secret. Generate a real random value (e.g. \`openssl rand -hex 32\`).`; + return null; +} + +function checkCriticalSecrets( + problems: SelfHostPreflightProblem[], + env: SelfHostPreflightEnv, +): void { + const seenValues = new Map(); // value -> first var name that used it + for (const name of CRITICAL_SECRET_VARS) { + const value = nonBlank(env[name]); + if (!value) continue; // presence is each caller's own concern; this only judges strength when SET + const problem = criticalSecretProblem(name, value); + if (problem) { + addProblem(problems, name, problem); + continue; + } + const firstSeenBy = seenValues.get(value); + if (firstSeenBy) + addProblem( + problems, + name, + `${name} must not reuse the same value as ${firstSeenBy} — each credential grants a distinct role, and a shared value lets one leaked/forged credential impersonate every role that reuses it.`, + ); + else seenValues.set(value, name); + } +} + export function preflightEnv(env: SelfHostPreflightEnv): SelfHostPreflightResult { const problems: SelfHostPreflightProblem[] = []; @@ -145,6 +206,8 @@ export function preflightEnv(env: SelfHostPreflightEnv): SelfHostPreflightResult "Set DATABASE_URL to a valid postgres:// URL with a database name, or leave it unset to use the SQLite backend.", ); + checkCriticalSecrets(problems, env); + return problems.length === 0 ? { ok: true, problems: [] } : { ok: false, problems }; } diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 720b8a78eb..e8fd880b6d 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -2376,7 +2376,7 @@ describe("queue processors", () => { title: "Clean PR", mode: "block", byok: false, provider: null, model: null, aiReviewAllAuthors: false, aiReviewCloseConfidence: undefined, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, baseSha: null, reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], - profile: null, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], changedPaths: ["src/a.ts"], + profile: null, securityFocus: false, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], changedPaths: ["src/a.ts"], features: { grounding: false, rag: false, enrichment: false, reputation: false }, }), }, @@ -2484,6 +2484,7 @@ describe("queue processors", () => { baseSha: null, reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], profile: null, + securityFocus: false, inlineComments: false, pathInstructions: [], pathGuidance: "", @@ -2548,7 +2549,7 @@ describe("queue processors", () => { title: "Current PR", mode: "block", byok: false, provider: null, model: null, aiReviewAllAuthors: false, aiReviewCloseConfidence: undefined, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, baseSha: null, reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], - profile: null, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], changedPaths: ["src/a.ts"], + profile: null, securityFocus: false, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], changedPaths: ["src/a.ts"], features: { grounding: false, rag: false, enrichment: false, reputation: false }, }), }, @@ -2597,7 +2598,7 @@ describe("queue processors", () => { title: "Partially published PR", mode: "block", byok: false, provider: null, model: null, aiReviewAllAuthors: false, aiReviewCloseConfidence: undefined, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, baseSha: null, reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], - profile: null, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], changedPaths: ["src/a.ts"], + profile: null, securityFocus: false, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], changedPaths: ["src/a.ts"], features: { grounding: false, rag: false, enrichment: false, reputation: false }, }), }, @@ -2649,7 +2650,7 @@ describe("queue processors", () => { title: "Current PR", mode: "block", byok: false, provider: null, model: null, aiReviewAllAuthors: false, aiReviewCloseConfidence: undefined, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, baseSha: null, reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], - profile: null, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], changedPaths: ["src/a.ts"], + profile: null, securityFocus: false, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], changedPaths: ["src/a.ts"], features: { grounding: false, rag: false, enrichment: false, reputation: false }, }), }, diff --git a/test/unit/selfhost-preflight.test.ts b/test/unit/selfhost-preflight.test.ts index a2c7a3c3c9..4302d069fc 100644 --- a/test/unit/selfhost-preflight.test.ts +++ b/test/unit/selfhost-preflight.test.ts @@ -66,7 +66,7 @@ describe("self-host environment preflight (#2080)", () => { expect( preflightEnv({ REDIS_URL: "redis://redis:6379", - SELFHOST_SETUP_TOKEN: "setup-secret", + SELFHOST_SETUP_TOKEN: "setup-secret-with-enough-entropy-1", PUBLIC_API_ORIGIN: "https://selfhost.example", }), ).toEqual({ ok: true, problems: [] }); @@ -88,7 +88,7 @@ describe("self-host environment preflight (#2080)", () => { ]) { const result = preflightEnv({ REDIS_URL: "redis://redis:6379", - SELFHOST_SETUP_TOKEN: "setup-secret", + SELFHOST_SETUP_TOKEN: "setup-secret-with-enough-entropy-1", PUBLIC_API_ORIGIN, }); @@ -108,7 +108,7 @@ describe("self-host environment preflight (#2080)", () => { ]) { const result = preflightEnv({ REDIS_URL, - SELFHOST_SETUP_TOKEN: "setup-secret", + SELFHOST_SETUP_TOKEN: "setup-secret-with-enough-entropy-1", PUBLIC_API_ORIGIN: "https://selfhost.example", }); @@ -202,10 +202,108 @@ describe("self-host environment preflight (#2080)", () => { expect(JSON.stringify(secretBearing)).not.toContain("super-secret-db"); }); + describe("critical secrets (Codex security finding: shipped placeholder tokens)", () => { + const baseEnv = { + REDIS_URL: "redis://redis:6379", + GITHUB_APP_ID: "123", + GITHUB_APP_PRIVATE_KEY: privateKey, + }; + + it("rejects each critical secret when it is still the exact placeholder shipped in .env.selfhost.example / .env.example", () => { + for (const [name, placeholder] of [ + ["GITHUB_WEBHOOK_SECRET", "change-this-long-random-value"], + ["GITTENSORY_API_TOKEN", "change-this-32-byte-random-token"], + ["GITTENSORY_MCP_TOKEN", "change-this-32-byte-random-token"], + ["INTERNAL_JOB_TOKEN", "change-this-32-byte-random-token"], + ["SELFHOST_SETUP_TOKEN", "change-this-long-random-value"], + ] as const) { + const result = preflightEnv({ ...baseEnv, [name]: placeholder }); + expect(result.ok).toBe(false); + expect(result).toEqual({ + ok: false, + problems: [expect.objectContaining({ var: name })], + }); + if (!result.ok) expect(JSON.stringify(result.problems)).not.toContain(placeholder); + } + }); + + it("rejects a critical secret that is non-blank but shorter than the minimum safe length", () => { + const result = preflightEnv({ ...baseEnv, GITHUB_WEBHOOK_SECRET: "weakvalue123" }); + expect(result).toEqual({ + ok: false, + problems: [expect.objectContaining({ var: "GITHUB_WEBHOOK_SECRET", message: expect.stringContaining("too short") })], + }); + expect(JSON.stringify(result)).not.toContain("weakvalue123"); + }); + + it("accepts a critical secret at exactly the minimum length, and one character below it still fails", () => { + const exactly20 = "a".repeat(20); + expect(preflightEnv({ ...baseEnv, GITHUB_WEBHOOK_SECRET: exactly20 })).toEqual({ ok: true, problems: [] }); + + const nineteen = "a".repeat(19); + const result = preflightEnv({ ...baseEnv, GITHUB_WEBHOOK_SECRET: nineteen }); + expect(result).toEqual({ + ok: false, + problems: [expect.objectContaining({ var: "GITHUB_WEBHOOK_SECRET" })], + }); + }); + + it("does not require any critical secret to be present — only judges strength when one is set", () => { + expect(preflightEnv(baseEnv)).toEqual({ ok: true, problems: [] }); + }); + + it("rejects two critical secrets that reuse the identical value, without echoing it", () => { + const sharedSecret = "a-perfectly-strong-random-value-1234"; + const result = preflightEnv({ + ...baseEnv, + GITTENSORY_API_TOKEN: sharedSecret, + GITTENSORY_MCP_TOKEN: sharedSecret, + }); + expect(result).toEqual({ + ok: false, + problems: [ + expect.objectContaining({ + var: "GITTENSORY_MCP_TOKEN", + message: expect.stringContaining("must not reuse the same value as GITTENSORY_API_TOKEN"), + }), + ], + }); + expect(JSON.stringify(result)).not.toContain(sharedSecret); + }); + + it("accepts every critical secret when each is a distinct, sufficiently long real value", () => { + expect( + preflightEnv({ + ...baseEnv, + GITHUB_WEBHOOK_SECRET: "webhook-secret-value-with-plenty-of-entropy", + GITTENSORY_API_TOKEN: "api-token-value-with-plenty-of-entropy-2", + GITTENSORY_MCP_TOKEN: "mcp-token-value-with-plenty-of-entropy-3", + INTERNAL_JOB_TOKEN: "internal-job-token-with-plenty-of-entropy-4", + SELFHOST_SETUP_TOKEN: "setup-token-value-with-plenty-of-entropy-5", + }), + ).toEqual({ ok: true, problems: [] }); + }); + + it("collects a placeholder/weak-secret problem for EVERY affected critical secret, not just the first", () => { + const result = preflightEnv({ + ...baseEnv, + GITHUB_WEBHOOK_SECRET: "change-this-long-random-value", + GITTENSORY_API_TOKEN: "change-this-32-byte-random-token", + }); + expect(result).toEqual({ + ok: false, + problems: [ + expect.objectContaining({ var: "GITHUB_WEBHOOK_SECRET" }), + expect.objectContaining({ var: "GITTENSORY_API_TOKEN" }), + ], + }); + }); + }); + it("flags blank values and invalid DATABASE_URL while never echoing supplied secrets", () => { const result = preflightEnv({ REDIS_URL: " ", - SELFHOST_SETUP_TOKEN: "secret-setup-token", + SELFHOST_SETUP_TOKEN: "secret-setup-token-with-enough-entropy", PUBLIC_API_ORIGIN: "https://selfhost.example", DATABASE_URL: "sqlite:///tmp/gittensory.sqlite?password=super-secret-db", }); @@ -218,7 +316,7 @@ describe("self-host environment preflight (#2080)", () => { ], }); const serialized = JSON.stringify(result); - expect(serialized).not.toContain("secret-setup-token"); + expect(serialized).not.toContain("secret-setup-token-with-enough-entropy"); expect(serialized).not.toContain("super-secret-db"); expect(serialized).not.toContain("sqlite:///tmp"); });