Skip to content

fix(selfhost): exclude Docker Compose's reserved _FILE vars from the secret-dereference scan - #4729

Merged
JSONbored merged 1 commit into
mainfrom
fix/compose-file-secret-false-error
Jul 10, 2026
Merged

fix(selfhost): exclude Docker Compose's reserved _FILE vars from the secret-dereference scan#4729
JSONbored merged 1 commit into
mainfrom
fix/compose-file-secret-false-error

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • loadFileSecrets() scans every env var ending in _FILE and dereferences it as a gittensory Docker-secret pointer. It had no exclusion for Docker Compose's own reserved COMPOSE_FILE (a colon-delimited list of compose file paths, e.g. docker-compose.yml:docker-compose.override.yml:docker-compose.local-gpu.yml) or COMPOSE_ENV_FILE — neither is ever a single readable file, so readFileSync always throws, logging a guaranteed-false level:"error" structured line on every container boot. Confirmed live on edge-nl-01 tonight ({"level":"error","event":"selfhost_secret_file_unreadable","var":"COMPOSE_FILE"} on every deploy).
  • This undermines the convention this codebase relies on: level:"error" logs are the actual signal for Sentry-visible operator alerts. A guaranteed false alarm on every restart trains operators to ignore them.
  • Fix: exclude both Compose-reserved _FILE vars by name before the dereference attempt.
  • Also extracted loadFileSecrets into its own module (src/selfhost/load-file-secrets.ts), injecting env/readFile as optional params (defaulting to process.env/real readFileSync, byte-identical at runtime) — server.ts itself boots the entire app on import and is Codecov-ignored, so there was no way to give this fix a real runtime regression test while it lived there.

Closes #4403

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves (e.g. Closes #123) — a linked open issue is required for every contributor PR.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally; codecov/patch requires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries — 7 tests, 100% stmt/branch coverage on the new module, including the exact regression repro (COMPOSE_FILE set → no dereference attempt, no error log).
  • npm run engine-parity:drift-check — clean (this file isn't a hand-duplicated twin).

If any required check was skipped, explain why:

  • No UI/MCP/OpenAPI/wrangler-binding/build-pipeline surface touched — not applicable. test:coverage was run targeted on the new test file (7/7 passing, 100% line/branch/function coverage on src/selfhost/load-file-secrets.ts) rather than the full unsharded suite.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed. (Ran the diff through the repo's own hasGenericSecretAssignment detector directly — clean.)
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics. (N/A — no public-facing text changed)
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. (N/A)
  • API/OpenAPI/MCP behavior is updated and tested where needed. (N/A)
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. (N/A)
  • Visible UI changes include a UI Evidence section below. (N/A — no visible UI change)
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs. (N/A)

UI Evidence

N/A — no UI/frontend/docs-visible change.

Notes

  • Verified against real production evidence: this exact false error has been firing on every edge-nl-01 boot tonight (multiple deploys observed during unrelated work).

…secret-dereference scan (#4403)

loadFileSecrets() treats every *_FILE env var as a gittensory Docker-secret
pointer, including Compose's own reserved COMPOSE_FILE (a colon-delimited
list of compose file paths, never a single readable file) and
COMPOSE_ENV_FILE. Both threw on every readFileSync attempt, logging a
guaranteed-false level:error on every container boot -- undermining the
convention that error-level logs are real operator-alertable signals.

Also extracts loadFileSecrets into its own module: server.ts boots the
whole app on import and is Codecov-ignored, so it had no way to carry a
real runtime regression test for this fix.
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.21%. Comparing base (bfc34cd) to head (c985055).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4729   +/-   ##
=======================================
  Coverage   94.21%   94.21%           
=======================================
  Files         439      440    +1     
  Lines       38704    38714   +10     
  Branches    14101    14104    +3     
=======================================
+ Hits        36466    36476   +10     
  Misses       1576     1576           
  Partials      662      662           
Files with missing lines Coverage Δ
src/selfhost/load-file-secrets.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.

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

loopover-orb Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-10 21:56:16 UTC

3 files · 1 AI reviewer · no blockers · readiness 100/100 · CI green · clean

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review: This PR changes guardrail-protected path(s): src/selfhost/load-file-secrets.ts (matched src/selfhost/**).

Review summary
This correctly extracts loadFileSecrets() out of server.ts into its own testable module and adds a targeted exclusion set for Docker Compose's reserved COMPOSE_FILE and COMPOSE_ENV_FILE vars, which previously always failed readFileSync and logged a false level:"error" alert on every container boot. The fix is applied at the right layer — inside the function that does the dereference, before the read is attempted — not as a downstream symptom patch, and the injected env/readFile params are byte-identical to the old hardcoded behavior at runtime. Tests cover both excluded vars, the real-secret dereference path, the already-set-value skip, the error/log path, and the default-args path, giving this genuine runtime coverage that server.ts (Codecov-ignored, boots the whole app) never had.

Nits — 7 non-blocking
  • src/selfhost/load-file-secrets.ts:1-13 — the doc comments are quite dense (three separate long comment blocks for a ~25-line function); consider trimming to the non-obvious parts (why COMPOSE_FILE/COMPOSE_ENV_FILE specifically, and why env/readFile are injectable) and dropping the restated 'extracted from server.ts' history now that it's captured in the PR description.
  • src/selfhost/load-file-secrets.ts:13 — COMPOSE_RESERVED_FILE_VARS is a fixed two-entry allowlist; if Compose ever reserves another _FILE-suffixed var this silently regresses back to a false error log, so a code comment pointing at Compose's docs as the source of truth would help future maintainers keep it in sync.
  • I could not see the server.ts call site for loadFileSecrets() in the truncated diff/file content, so I can't directly confirm it's still invoked with no args post-refactor — worth double-checking, though green build+boot-smoke CI makes a missed call site unlikely.
  • Consider a short unit test asserting the exported COMPOSE_RESERVED_FILE_VARS (or equivalent behavior) rejects any future Compose-reserved var if one gets added to the set, so the allowlist doesn't drift unnoticed.
  • Since this is the first module extracted this way, note the injectable-defaults pattern (env/readFile params) in CONTRIBUTING.md or a comment there if you expect other server.ts helpers to be pulled out similarly for testability.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #4403
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: 48 registered-repo PR(s), 40 merged, 285 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 48 PR(s), 285 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 48 PR(s), 285 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
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.

🟩 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 Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 10, 2026
@JSONbored
JSONbored merged commit eafcd35 into main Jul 10, 2026
12 checks passed
@JSONbored
JSONbored deleted the fix/compose-file-secret-false-error branch July 10, 2026 21:57
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. manual-review Gittensor contributor context

Development

Successfully merging this pull request may close these issues.

[Bug]: loadFileSecrets() misfires on Docker Compose's own COMPOSE_FILE, logging a false error every boot

1 participant