Skip to content

fix(selfhost): reject known-placeholder and weak critical secrets at boot - #2686

Merged
JSONbored merged 2 commits into
mainfrom
fix/selfhost-starter-placeholder-secrets
Jul 3, 2026
Merged

fix(selfhost): reject known-placeholder and weak critical secrets at boot#2686
JSONbored merged 2 commits into
mainfrom
fix/selfhost-starter-placeholder-secrets

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

Fixes a critical Codex security finding: .env.selfhost.example shipped ENABLED (not commented-out) placeholder values for GITHUB_WEBHOOK_SECRET, GITTENSORY_API_TOKEN, GITTENSORY_MCP_TOKEN, INTERNAL_JOB_TOKEN, and SELFHOST_SETUP_TOKEN. The self-hosting quickstart docs instruct operators to cp .env.selfhost.example .env. An operator who copies the file and misses "fill in the placeholders" runs an instance with:

  • A publicly known webhook HMAC secretverifyGitHubSignature accepts any payload signed with the well-known string, so an attacker can forge valid-looking GitHub webhook deliveries.
  • A publicly known GITTENSORY_API_TOKEN — authenticates as the server-to-server API actor, bypasses app-role checks, and is exempt from per-repo write-permission checks (authenticatePrivateToken, requireRepoWriteAccess).
  • A publicly known INTERNAL_JOB_TOKEN — gates internal-only routes.

All of this fails silently: nothing in the current preflight check or runtime path rejects these known values.

Fix (defense in depth — two independent layers)

  1. Boot-time preflight now rejects the vulnerability class, not just this instance of it. src/selfhost/preflight.ts's preflightEnv (already gates server.ts's main(), throwing before the process starts serving) now validates all five critical secrets whenever they're set:

    • Exact match against the known-placeholder strings shipped in .env.selfhost.example / .env.example → reject.
    • Shorter than 20 characters (a real openssl rand -hex 32 value is 64 chars) → reject.
    • Identical to another one of the five critical secrets → reject (an operator who reuses one string across all of them collapses every role into the highest-privilege one that matches first).

    Presence is intentionally not newly required — most of these gate optional features (MCP, internal routes) and I didn't want to risk breaking a legitimate not-yet-fully-configured boot. This closes the actual reported hole (a known-bad value silently accepted) without expanding scope into a "what should be mandatory" redesign.

  2. .env.selfhost.example no longer ships an enabled placeholder for any of the five. They're now commented out with explicit per-secret generation guidance (openssl rand -hex 32). A naive cp && docker compose up with an unedited file now refuses to boot with a clear, itemized preflight error instead of silently running with known-compromised credentials.

Also updated the quickstart doc callout (the exact page the finding cited as instructing users toward the vulnerable path) to explicitly call out generating distinct random values for each secret.

Scope

  • Stayed within wantedPaths
  • No secrets/wallets/hotkeys/trust-scores/reward values anywhere (error messages never echo the supplied value — verified by test)
  • Regenerated apps/gittensory-ui/src/lib/selfhost-env-reference.ts (npm run selfhost:env-reference) since the source line numbers in preflight.ts shifted

Validation

  • npm run typecheck
  • npm run test:coverage (unsharded) — 100% line+branch coverage on every changed line in src/selfhost/preflight.ts, verified by diffing changed lines against the v8 coverage map directly
  • npm run selfhost:env-reference:check
  • npm audit --audit-level=moderate — 0 vulnerabilities
  • npm run ui:test, npm run ui:lint (fixed one prettier violation in the docs page), npm run actionlint, npm run db:migrations:check
  • New tests in test/unit/selfhost-preflight.test.ts: rejects each critical secret at the exact placeholder value; rejects a too-short value without echoing it; boundary test at exactly/one-under the minimum length; presence still optional; rejects two secrets sharing an identical value; accepts five distinct sufficiently-long real values; collects a problem for every affected secret, not just the first

Two pre-existing, environment-specific failures unrelated to this change (test/unit/check-schema-drift-script.test.ts's tsx binary path issue, ui:typecheck's missing node:process/@lovable.dev types) reproduce identically on a clean main checkout in this worktree and are excluded above.

Note: this branch also carries a small unrelated commit (fix(review): add the missing securityFocus field to 5 AI-review-cache test fixtures) cherry-picked from #2684, needed to get a green npm run typecheck locally since main is currently broken for anyone branching fresh off it. Will rebase to drop the duplicate once #2684 merges.

Safety

  • No secrets in code/tests/PR text — all new tests use obviously-fake placeholder-style strings, never anything resembling a real credential
  • Backward compatible: an operator who already generated real, distinct, sufficiently-long secrets sees no behavior change
  • Fails closed: a rejected secret prevents boot entirely rather than degrading into a partially-working state

JSONbored added 2 commits July 2, 2026 22:26
… test fixtures

#2675 (feat(review): add a security-focused review profile toggle) added
securityFocus as a required AiReviewCacheInput field after these fixtures
(from #2667) were written, breaking npm run typecheck on main for anyone
branching fresh off it.
…boot

.env.selfhost.example shipped ENABLED (not commented-out) placeholder values
for GITHUB_WEBHOOK_SECRET, GITTENSORY_API_TOKEN, GITTENSORY_MCP_TOKEN,
INTERNAL_JOB_TOKEN, and SELFHOST_SETUP_TOKEN. An operator who copies the
starter to .env per the quickstart docs and misses "fill in the placeholders"
runs an instance with a PUBLICLY KNOWN webhook HMAC secret (forgeable
signatures) and PUBLICLY KNOWN static bearer tokens -- GITTENSORY_API_TOKEN
authenticates as the server-to-server actor and bypasses per-repo write
checks, INTERNAL_JOB_TOKEN gates internal routes -- silently, with no error
at boot or runtime.

- The boot-time preflight check (already gates server.ts's main(), throwing
  before the process starts serving) now rejects any of the five critical
  secrets that is set to the exact known-placeholder string, or that is
  merely too short to be a real generated secret, or that duplicates another
  critical secret's value. Presence is still each secret's own concern (most
  are feature-gating, not universally required) -- this only judges
  STRENGTH whenever one is actually set, so it can never be silently bypassed
  by leaving the file's placeholder in place.
- .env.selfhost.example now ships these five lines commented out, with
  explicit per-secret generation guidance, instead of enabled placeholders.
- The quickstart doc callout it directed users through now explicitly warns
  about generating distinct random values for each secret.

Defense in depth: the docs + example file guide an operator toward doing the
right thing, and the preflight check makes doing the wrong thing impossible
rather than merely discouraged.
@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui eb25275 Commit Preview URL

Branch Preview URL
Jul 03 2026, 05:40 AM

@JSONbored JSONbored self-assigned this Jul 3, 2026
@JSONbored
JSONbored merged commit 361d29c into main Jul 3, 2026
11 checks passed
@JSONbored
JSONbored deleted the fix/selfhost-starter-placeholder-secrets branch July 3, 2026 05:41
@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@00d1490). Learn more about missing BASE report.
⚠️ Report is 7 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2686   +/-   ##
=======================================
  Coverage        ?   96.12%           
=======================================
  Files           ?      238           
  Lines           ?    26709           
  Branches        ?     9686           
=======================================
  Hits            ?    25675           
  Misses          ?      424           
  Partials        ?      610           
Files with missing lines Coverage Δ
src/selfhost/preflight.ts 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant