fix(db): create credential store with owner-only permissions - #3381
Open
elvis460 wants to merge 1 commit into
Open
fix(db): create credential store with owner-only permissions#3381elvis460 wants to merge 1 commit into
elvis460 wants to merge 1 commit into
Conversation
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
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.
Summary
~/.9router/db/data.sqliteis created without an explicit mode, so it inherits the process umask —0644on a defaultumask 022, i.e. world-readable. That file is the credential store:providerConnections.data— provider OAuth access and refresh tokens, stored as plaintext JSONapiKeys.key— client API keys, stored in plaintext (sk-…, not hashed)Any other local account, or any process running under a different uid, can read that file and obtain full access to the linked AI provider accounts.
This is inconsistent with how the project already treats its other secrets.
src/cli/api/client.js:55writes the CLI secret with an explicit{ mode: 0o600 }, andjwt-secretlands at0600as well. The DB — strictly more sensitive than either of those — is the one credential store that inherits the umask instead.Reproduce
On a stock 0.5.55 install:
Schema-migration backups have the same problem, because
backupDbLite()lets SQLite create the attached file:Changes
src/lib/db/paths.js—ensureDirs()now creates directories with0700. AddshardenPermissions(), which chmods the data dir, db dir and backups dir to0700, anddata.sqliteplus its-wal/-shmsidecars to0600.src/lib/db/driver.js— callshardenPermissions()after adapter init, at the point where the DB file is guaranteed to exist.src/lib/db/backup.js— migration backup dirs created0700; backup files chmodded0600aftercopyFileSync/ATTACH.src/lib/dataDir.js— aDATA_DIRsupplied via env is created with0700.Three deliberate design points:
0644keeps that mode indefinitely.hardenPermissions()therefore runs on every startup and is idempotent.chmodthere is ACL-based and only the read-only bit maps through, so tightening is restricted to POSIX platforms.Verification
Verified against a live 0.5.55 install. After applying
0600/0700to an existing install, a full OpenAI (codex) OAuth re-authentication completed normally and wrote a freshproviderConnectionsrow (4064 bytes,isActive=1). File modes survived the write, since SQLite writes in place rather than recreating the file. Tightening the mode does not interfere with provider authentication — which is the obvious concern with a change like this, given that adding a provider is a write path.Adds
tests/unit/db-file-permissions.test.jscovering:0700, DB06000755/0644install → repaired on startup06000700, file0600Skipped on Windows.
Out of scope
Encrypting credentials at rest is a larger design change (key management, migration of existing rows) and is intentionally not attempted here. Filesystem permissions are the cheap first line of defence regardless of whether encryption is added later.