fix(security): private settings permissions + hash apiKey cache key - #267
Open
raymondginger2018-sudo wants to merge 3 commits into
Open
Conversation
Two P0 hardening fixes:
1. settings.json now written with user-only permissions
- New core/common/private-storage.ts: writePrivateFile (0600 on POSIX,
icacls /inheritance:r + /grant:r <user>:F on Windows) and
ensurePrivateDirectory (0700 / current-user ACL).
- writeSettingsFile routes through these helpers; previously the API key
landed in ~/.deepcode/settings.json with the default permissive mode /
profile ACL (Authenticated Users can read it on Windows).
2. openai-client no longer keeps the raw apiKey in module state
- Cache key is now sha256(apiKey)[:16] + baseURL instead of the plaintext
apiKey, so a heap dump or crash report never contains the secret.
Tests (private-storage.test.ts): POSIX 0600/0700 modes, Windows ACL
restriction, idempotency. Full suite: 284 tests, 0 failures.
P1/P2 hardening on top of the private-settings fix:
1. Atomic session writes (session.ts)
- saveSessionsIndex / saveSessionMessages now write via
writeFileAtomic (tmp + rename) instead of writeFileSync, so a crash
mid-write never leaves a truncated sessions-index.json / .jsonl.
2. MCP error classification (mcp-manager.ts)
- classifyMcpError() tags tool failures as timeout/connection/auth/
protocol/busy so the agent can distinguish a wedged server (retry)
from a config error (don't retry). Surface as mcpErrorKind on the
tool result.
3. Background log sweep (bash-handler.ts)
- sweepOldBackgroundLogs() removes deepcode-background/*.log older than
7 days on each background start (was never cleaned).
Evaluated and left as-is: sessionWorkingDirs is already cleared on session
delete; deps already use package-lock.json + npm ci in CI.
Tests: hardening.test.ts (atomic write, classifyMcpError, sweep) — 9 new
cases. Full suite: 293 tests, 286 pass, 0 fail. tsc --noEmit clean.
Author
|
Extended this PR with P1/P2 hardening (commits added to the branch): P1
P2
Evaluated, no change needed
Full core suite: 293 tests, 286 pass, 0 fail (7 pre-existing skips). clean. |
…le-response total activeTokens was set to getTotalTokens(responseUsage), i.e. the token count of only the latest response. Since each individual call stays below the autoCompactWindow threshold, session.activeTokens never crosses it and auto-compaction never triggers. Long sessions keep resending their full history on every turn, causing runaway token usage. Fix: - activeTokens now accumulates: (entry.activeTokens ?? 0) + getTotalTokens(responseUsage) - reset activeTokens to 0 after compaction so it does not re-trigger immediately Add regression tests (auto-compact.test.ts) and update the two existing session tests that asserted the old single-response behavior.
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
1 participant
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.
Two P0 hardening fixes:
1. settings.json written with user-only permissions
~/.deepcode/settings.jsonholds the API key but was written with the default permissive mode (POSIX umask) / profile ACL. On Windows,Authenticated Userscan read it by default.core/common/private-storage.ts:writePrivateFile— 0600 on POSIX;icacls /inheritance:r+/grant:r <user>:F(current-user exclusive) on Windows.ensurePrivateDirectory— 0700 / current-user ACL.writeSettingsFilenow routes through these helpers.2. openai-client no longer keeps the raw apiKey in module state
The client cache key was
\`${settings.apiKey}::${settings.baseURL}\— the raw secret lingered in a module-level variable, so a heap dump or crash report contained it verbatim.sha256(apiKey)[:16]::baseURL.Tests
packages/core/src/tests/private-storage.test.ts— POSIX 0600/0700 modes, Windows ACL restriction, idempotency.Full core suite: 284 tests, 277 pass, 0 fail (7 pre-existing skips).
tsc --noEmitclean.