Skip to content

fix(selfhost): make Docker Compose secret files readable by the container#5124

Merged
JSONbored merged 1 commit into
mainfrom
fix/secrets-mount-uid-mismatch
Jul 11, 2026
Merged

fix(selfhost): make Docker Compose secret files readable by the container#5124
JSONbored merged 1 commit into
mainfrom
fix/secrets-mount-uid-mismatch

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • Follow-up to feat(selfhost): wire native Docker Compose secrets for the gittensory service #5121: the first real deploy of the Docker Compose secrets feature (edge-nl-01) surfaced 5 selfhost_secret_file_unreadable errors (Sentry-visible — level:"error" + an event field) immediately at boot.
  • Root cause: scripts/selfhost-init-secrets.sh created each placeholder file chmod 600. Standalone (non-Swarm) Docker Compose secrets: is a plain bind mount under the hood — it cannot remap in-container ownership, so the container reads the file as its own uid (the Dockerfile's USER node, uid 1000), which is essentially never the deploying host user's uid. An owner-only 600 file is therefore unreadable to the app itself.
  • I initially tried the "correct-looking" fix — explicit uid/gid/mode on the service-level secrets: entries — but confirmed against a real Compose v5.3.1 daemon on the actual affected server that Compose silently ignores these fields for file-backed secrets ("secrets uid, gid and mode are not supported, they will be ignored"). This is a known, longstanding Compose limitation (bind mounts can't implement per-mount uid remapping), not something fixable from the compose file.
  • The actual fix: 644 is the minimum permission that works without requiring the host to have a matching uid/group with the container. scripts/selfhost-init-secrets.sh now creates new placeholders at 644, and self-heals the mode on any already-existing-but-still-empty placeholder (checked via [ ! -s "$path" ]) — so a box that already deployed the broken 600 version (like edge-nl-01) fixes itself on the very next run, with zero manual intervention. A file that has been populated with a real secret is never touched (content or permissions).

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • 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.
  • Maintainer/own-branch PR — no linked issue; this is a same-session regression fix for feat(selfhost): wire native Docker Compose secrets for the gittensory service #5121, landed directly.

Validation

  • npm run test:ci — full local gate green.
  • npm audit --audit-level=moderate — 0 vulnerabilities.
  • Reproduced the exact failure on the real affected server: a uid: 1000 reader against a 600 file mounted via docker compose secrets:UNREADABLE.
  • Verified the fix on the same server: the identical file at 644READABLE.
  • Verified the self-heal logic locally: an existing empty 600 placeholder is corrected to 644 on the next script run; a populated 600 secret (real content) is left completely untouched, both content and mode.
  • Confirmed via a real docker compose invocation (not just docs) that explicit uid/gid/mode on file-backed secrets is silently ignored on Compose v5.3.1 — ruled out before choosing the 644 approach.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. (N/A — file-permission fix only.)
  • API/OpenAPI/MCP behavior is updated and tested where needed. (N/A.)
  • No UI change — UI Evidence section not applicable.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs. secrets/README.md updated to document the 644 tradeoff honestly (readable by any local host user, not just the deploying account — narrower than .env's typical 600, and an operator on a shared host should keep using inline .env instead if that's unacceptable).

…iner

Every secret file was created chmod 600, unreadable by the container's
node user (uid 1000) since standalone Compose secrets are a plain bind
mount that preserves the HOST file's raw ownership -- it cannot remap
uid/gid/mode the way Swarm secrets can (confirmed against a real
Compose v5.3.1 daemon: it warns "not supported" and silently no-ops
uid/gid/mode fields for file-backed secrets). Every secret read failed
with a Sentry-visible selfhost_secret_file_unreadable error on the
first real deploy.

644 is the minimum mode that works without requiring the host to have
a matching uid/group. scripts/selfhost-init-secrets.sh now also
self-heals the mode on an already-existing but still-empty placeholder
(never touches one that's been populated with a real value), so a box
that already deployed the broken 600 version fixes itself on the next
run. Verified against the actual affected server (edge-nl-01): 600 is
UNREADABLE to a uid-1000 reader, 644 is READABLE.
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.32%. Comparing base (d8f938e) to head (5c8d5ed).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5124   +/-   ##
=======================================
  Coverage   94.32%   94.32%           
=======================================
  Files         471      471           
  Lines       39821    39821           
  Branches    14533    14533           
=======================================
  Hits        37560    37560           
  Misses       1583     1583           
  Partials      678      678           
Flag Coverage Δ
shard-1 46.34% <ø> (ø)
shard-2 34.66% <ø> (ø)
shard-3 30.98% <ø> (ø)
shard-4 32.87% <ø> (ø)
shard-5 33.65% <ø> (ø)
shard-6 44.87% <ø> (ø)

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

gittensory-orb Bot commented Jul 11, 2026

Copy link
Copy Markdown

Warning

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

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-11 17:25:19 UTC

3 files · 2 AI reviewers · 2 blockers · readiness 93/100 · CI green · unstable

⏸️ Suggested Action - Manual Review

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

Review summary
This fixes a real production incident (5 selfhost_secret_file_unreadable errors on edge-nl-01) by changing placeholder secret files from 600 to 644, since standalone Compose secrets are bind mounts that can't remap uid ownership, making 600 files unreadable by the container's node user. The script now also self-heals already-deployed 600 placeholders via an `elif [ ! -s "$path" ]` check that only touches files still empty (size 0), leaving any populated secret's content and permissions untouched. The comment-heavy docs/compose changes accurately document a confirmed Compose v5.3.1 limitation and the tradeoff of 644 (world-readable on host, not visible via docker inspect), matching the described root cause and fix.

Nits — 6 non-blocking
  • scripts/selfhost-init-secrets.sh: the self-heal branch trusts `[ ! -s "$path" ]` (zero bytes) as the sole signal a file is still a placeholder — an operator who wrote a secret value and then truncated/cleared it back to empty (e.g., mid-edit) would silently get their file re-chmod'd to 644, though this is a narrow edge case given the file is empty anyway.
  • secrets/README.md: worth explicitly noting the multi-tenant-host caveat (644 = readable by any local user with shell access) directly in the 'Files' section too, not just the tradeoff callout, since that's the section operators are most likely to skim when actually adding a secret.
  • Consider logging which specific files were mode-healed (not just a count) in scripts/selfhost-init-secrets.sh, to make it easier to audit exactly which boxes had the broken 600 state during the transition period.
  • docker-compose.yml: since Compose confirmed silently ignoring uid/gid/mode on secrets, consider filing an upstream note or version-pinning comment so a future Compose release that starts supporting these fields doesn't reintroduce the same failure mode via a well-intentioned 'let's pin the mode properly' PR.
  • nit: secrets/README.md:49 should call out the upgrade case explicitly: anyone who already populated a file while following the old 600 guidance must manually chmod it to 644 or switch back to inline env, because the script intentionally will not heal non-empty files.
  • Diff looks like trivial or whitespace-only churn — Reduce whitespace-only or formatting-only churn and keep the diff focused on substantive changes.

Concerns raised — review before merging

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
Signal Result Evidence
Code review ❌ 2 blockers 2 reviewers, synthesized
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
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 (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 45 registered-repo PR(s), 37 merged, 414 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 45 PR(s), 414 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Improvement ⚠️ ℹ️ Insufficient signal risk: low · value: insufficient-signal — Nothing measurable for the structural-improvement analyzers on this PR (e.g. no code files changed). LLM value judgment: moderate — This is a targeted, well-verified fix for a confirmed production incident (real Sentry errors on a real host) with a self-healing migration path, not speculative hardening.
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: 45 PR(s), 414 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
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.
[BETA] Chat with Gittensory

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

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

Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands

🟩 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

@gittensory-orb gittensory-orb Bot added the manual-review Gittensor contributor context label Jul 11, 2026
@JSONbored JSONbored merged commit a416632 into main Jul 11, 2026
19 checks passed
@JSONbored JSONbored deleted the fix/secrets-mount-uid-mismatch branch July 11, 2026 17:25
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant