Conversation
config.db stores OAuth access/refresh tokens and DCR client secrets as plain JSON, but was created with mode 0644 and backed up with 0644. - Open the database 0600. - Tighten an existing group/world-readable database on open, clearing only the 0o077 bits so a stricter owner mode is never widened. This is best-effort: failures are logged at debug level and never block startup. - Stage Backup in an owner-only temp file next to the destination and rename it into place. bbolt's Tx.CopyFile opens with O_CREATE|O_TRUNC, so its mode argument is ignored for an existing destination and the database bytes would land in whatever permissions a stale backup carried. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Deploying mcpproxy-docs with
|
| Latest commit: |
0ca3f08
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://935f6982.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://fix-db-file-mode-0600.mcpproxy-docs.pages.dev |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 35746068151 --repo smart-mcp-proxy/mcpproxy-go
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
~/.mcpproxy/config.dbstores OAuth access/refresh tokens and DCR client secrets as plain JSON (internal/storage/models.go), but was created with mode0644and backed up with0644. This PR makes the database and its backups owner-only.bbolt.Opennow passes0600.0o077bits so a stricter owner mode is never widened. The mode argument tobbolt.Openapplies only at creation and bbolt never chmods an existing file, so this migration is the entire fix for anyone who already has a database.Backupno longer usesTx.CopyFiledirectly.CopyFileopens the destination withO_CREATE|O_TRUNC, so its mode argument is ignored when the file already exists — a chmod afterwards comes too late, since the database bytes would already have landed in whatever permissions a stale backup carried. The copy is now staged in an owner-only temp file next to the destination and renamed into place.Why
Scoped honestly:
~/.mcpproxyis created0700(internal/config/loader.go), so in a default install another local user cannot traverse to the0644database. This is defense-in-depth, and the 0700 directory is not an invariant — theMkdirAllis skipped whenDataDirstill contains an unresolved${...}, andMkdirAllnever tightens an already-existing directory.Backupcurrently has no production callers — it is reachable only as an exported method declared onappctx.StorageInterface. The0644copy was therefore a latent hazard on a public API, not a live leak.Out of scope, deliberately:
cmd/mcpproxy/db_cmd.gocompact/repair propagate the on-disk mode on purpose (info.Mode().Perm(), with a comment explaining bbolt will not chmod an existing file). They become correct by propagation after this change and are untouched.~/.mcpproxy/index.bleve/and the log files (documented as0644) are not covered here.How verified
TDD: five new tests in
internal/storage/bbolt_file_mode_test.go(fresh create, migration of an existing0644database, owner-bit preservation, backup to a new destination, backup over a pre-existing0644destination), each confirmed failing before the fix. They are skipped on Windows, whereos.Chmodonly toggles the read-only bit andos.Statreports0666.Also verified against a real
mcpproxy serveinstance on a scratch data dir: a freshconfig.dblands-rw-------; loosening it to0644and restarting migrates it back withTightened permissions on ... from 0644 to 0600at debug level; a third start on an already-0600database logs nothing.Cross-reviewed with
codex gpt-5.6-solover three rounds; two rounds returned blocking findings on the backup path (both verified againstgo.etcd.io/bbolt@v1.5.0/tx.gobefore fixing), the third returned APPROVE.🤖 Generated with Claude Code