Skip to content

docs(selfhost): describe what the starters actually ship for critical secrets - #6365

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
luciferlive112116:docs/preflight-placeholder-comment
Jul 16, 2026
Merged

docs(selfhost): describe what the starters actually ship for critical secrets#6365
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
luciferlive112116:docs/preflight-placeholder-comment

Conversation

@luciferlive112116

Copy link
Copy Markdown
Contributor

Summary

KNOWN_PLACEHOLDER_SECRETS's comment in src/selfhost/preflight.ts told the reader that .env.selfhost.example / .env.example "ship these EXACT literal placeholder values" for the webhook HMAC secret and the static API/MCP/internal bearer tokens. I checked both files: that was never true of either. A reader was being misled about which secrets ship exploitable defaults today.

Comment-only -- no executable line changes.

What the starters actually ship

Every CRITICAL_SECRET_VARS entry ships commented out in both files, so the real hazard is what an operator finds when they uncomment one:

File Shape
.env.selfhost.example All five commented and valueless (# GITHUB_WEBHOOK_SECRET=, L34-48). Uncommenting yields a blank -- checkCriticalSecrets skips a blank outright, and a blank cannot be a publicly known value.
.env.example Same, except SELFHOST_SETUP_TOKEN (L207), whose commented line carries change-this-long-random-value.

So the one real exposure is # SELFHOST_SETUP_TOKEN=change-this-long-random-value: uncommenting it is the obvious way to turn the first-run wizard on, and it hands the operator a published token unless this set stops them. That literal is also the string most likely to reach a var that ships valueless -- .env.example repeats it on POSTGRES_PASSWORD's commented line (L242), so it reads like the house placeholder rather than one var's. The comment now records exactly this.

On the issue's second question -- is the check dead for the four that ship valueless?

No, and the reason is worth recording, so I put it in the comment rather than dropping entries:

  • The set is matched against whatever the operator actually SET, not against what the files ship -- so it still fires wherever that literal is pasted, regardless of which var shipped it.
  • It is the only check that can catch it: change-this-long-random-value is 29 chars, which passes MIN_SECRET_LENGTH's 20-char bar, so the length heuristic below would wave it straight through.

change-this-32-byte-random-token ships in neither starter today. Noted inline and kept as defence-in-depth -- retiring a once-published placeholder buys nothing when the match is against operator input.

Scope

  • One coherent change; comment-only, in src/ (a wanted path).
  • No secrets/tokens/wallet/trust-score/reward terms.
  • No changelog, site/, CNAME, or lovable changes.

Validation

  • npm run typecheck -- 0 errors.
  • npx vitest run test/unit/selfhost-preflight.test.ts -- 18/18 pass (the only suite importing selfhost/preflight, confirmed by grep).
  • git diff --check -- clean.
  • Verified every changed line is a comment programmatically; verified each claim above by re-parsing both starter files at this branch's rebased base.
  • Rebased on latest upstream/main -- no base conflict.

Safety

  • Comment-only: no behaviour change, so no auth/CORS path is affected and no coverage surface is added.
  • No secrets committed. The placeholder strings quoted here are published starter-file values, already public in-repo -- naming them is the point of the check.

Closes #6285

… secrets

preflight.ts's KNOWN_PLACEHOLDER_SECRETS comment claimed
.env.selfhost.example / .env.example "ship these EXACT literal
placeholder values" for the webhook HMAC secret and the static
API/MCP/internal bearer tokens. Checked both files: that was never true of
either, and the real shape is worth stating precisely, since a reader was
being misled about which secrets ship exploitable defaults today.

Every CRITICAL_SECRET_VARS entry ships COMMENTED OUT in both files, so the
hazard is what an operator finds when they uncomment one. In
.env.selfhost.example all five are commented and valueless, which yields a
blank -- checkCriticalSecrets skips a blank outright, and a blank cannot be
a publicly known value. .env.example is the same except
SELFHOST_SETUP_TOKEN, whose commented line carries
change-this-long-random-value: uncommenting it, the obvious way to turn the
first-run wizard on, hands the operator a published token unless this set
stops them. The comment now records that, plus the fact that .env.example
repeats the same literal on POSTGRES_PASSWORD's commented line, which makes
it read like the house placeholder rather than one var's.

On whether the check is now dead for the four that ship valueless: it is
not. The set is matched against whatever the operator actually SET, not
against what the files ship, so it still fires wherever that literal is
pasted. It is also the ONLY check that can catch it -- at 29 characters it
passes MIN_SECRET_LENGTH's 20-char bar, so the length heuristic below would
wave it straight through.

change-this-32-byte-random-token ships in neither starter today; noted
inline and kept, since retiring a once-published placeholder buys nothing
when the match is against operator input.

Comment-only: no executable line changes.

Closes JSONbored#6285
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.60%. Comparing base (b7e1bec) to head (8fccb99).
⚠️ Report is 14 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6365   +/-   ##
=======================================
  Coverage   95.60%   95.60%           
=======================================
  Files         599      599           
  Lines       47221    47221           
  Branches    15024    15024           
=======================================
  Hits        45147    45147           
  Misses       1290     1290           
  Partials      784      784           
Flag Coverage Δ
shard-1 44.00% <ø> (-0.18%) ⬇️
shard-2 36.80% <ø> (+0.40%) ⬆️
shard-3 32.46% <ø> (-0.07%) ⬇️
shard-4 34.55% <ø> (-0.06%) ⬇️
shard-5 31.01% <ø> (-0.59%) ⬇️
shard-6 45.31% <ø> (+0.43%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/selfhost/preflight.ts 100.00% <ø> (ø)

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 16, 2026
@loopover-orb

loopover-orb Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-16 05:14:18 UTC

1 file · 1 AI reviewer · no blockers · readiness 95/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This is a comment-only fix that corrects a misleading security comment in preflight.ts, replacing an inaccurate claim that starter files ship EXACT literal placeholder secrets with an accurate description of what each starter file actually ships (commented-out and mostly valueless, except SELFHOST_SETUP_TOKEN in .env.example). No executable lines changed, so behavior is identical; the corrected comment is well-reasoned and traces the actual risk (the one literal that ships, why the exact-match set still matters even for valueless vars, and why MIN_SECRET_LENGTH alone wouldn't catch it). The claims in the comment about file contents (L34-48, L207, L242) are not independently verifiable from this diff alone since those files aren't shown, but the logic is internally consistent with the code present.

Nits — 5 non-blocking
  • The comment is quite long (roughly 20 lines) for a single constant declaration; consider trimming to the load-bearing facts and moving the historical/issue-tracking narrative to the PR description or a shorter reference.
  • No test changes accompany this PR, which is appropriate since it's comment-only, but confirm the repo's coverage tooling doesn't penalize this file for the lack of test-path evidence.
  • Consider linking to the actual .env.example / .env.selfhost.example lines (or asserting on their content in a lightweight test) so future edits to those files can't silently re-introduce the drift this PR is fixing.
  • If the comment's factual claims about the starter files are important to keep in sync, a simple test asserting SELFHOST_SETUP_TOKEN's commented default equals the KNOWN_PLACEHOLDER_SECRETS entry would guard against future drift.
  • Code changes lack test evidence — Add focused regression tests or explain why existing coverage is sufficient.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #6285
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 141 registered-repo PR(s), 79 merged, 32 issue(s).
Contributor context ✅ Confirmed Gittensor contributor luciferlive112116; Gittensor profile; 141 PR(s), 32 issue(s).
Improvement ℹ️ None detected risk: low · value: none · LLM: minor
Linked issue satisfaction

Addressed
The PR rewrites the misleading comment to accurately reflect that only SELFHOST_SETUP_TOKEN ships a literal placeholder while the other four secrets ship blank/commented, and it explicitly explains why the KNOWN_PLACEHOLDER_SECRETS check still matters (it matches against operator-set values, not shipped defaults, and catches the 29-char literal that bypasses MIN_SECRET_LENGTH).

Review context
  • Author: luciferlive112116
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 141 PR(s), 32 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Triage stale or unlinked PRs.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 0b0f451 into JSONbored:main Jul 16, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs(selfhost): preflight.ts's "ships EXACT literal placeholder" comment no longer matches .env.example for 4 of 5 named secrets

1 participant