feat: load secrets from systemd credentials ($CREDENTIALS_DIRECTORY)#743
feat: load secrets from systemd credentials ($CREDENTIALS_DIRECTORY)#743MarvinFS wants to merge 2 commits into
Conversation
Add loadSystemdCredentials(), called once at the start of the worker main(). It copies any files systemd places in $CREDENTIALS_DIRECTORY into process.env (only when unset/empty; trailing newline trimmed; file name maps 1:1 to the env var), letting operators pass provider keys via systemd's native credential mechanism (LoadCredential=, LoadCredentialEncrypted=) instead of Environment=/EnvironmentFile=. Unlike the environment, credentials are decrypted into a private tmpfs and never placed in the process environment, so they do not appear in /proc/<pid>/environ. A value assigned to process.env after exec is readable in-process but is not written back to the exec environ block (verified on Node 22/glibc), so loading at runtime does not reintroduce the exposure. Fully opt-in and a no-op when $CREDENTIALS_DIRECTORY is unset, so non-systemd hosts and existing Environment=/.env setups are unaffected; explicitly set vars always win. Adds unit tests.
|
@MarvinFS is attempting to deploy a commit to the rohitg00's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughAdds a new ChangesSystemd Credentials Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/index.ts (1)
161-163: 💤 Low valueDrop the startup behavior comment here.
This is another WHAT-style source comment. The call site is self-explanatory once
loadSystemdCredentials()is named clearly; keep the operational detail in docs if it needs to be preserved.As per coding guidelines,
src/**/*.{ts,js}: No code comments explaining WHAT — use clear naming instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/index.ts` around lines 161 - 163, Remove the WHAT-style startup behavior comment block and rely on the clear function name loadSystemdCredentials() at the call site; delete the explanatory lines describing systemd-delivered secrets and the CREDENTIALS_DIRECTORY mechanics and leave the call (or a short one-line comment only if needed for intent like "load systemd credentials") so operational details remain in external docs rather than inline.src/load-credentials.ts (1)
4-24: 💤 Low valueMove the operator-facing behavior notes out of
src/.These comments explain what the loader does and how it works operationally. The function name is already clear; keep the code terse and move the systemd
/procrationale to README or operator docs instead.As per coding guidelines,
src/**/*.{ts,js}: No code comments explaining WHAT — use clear naming instead.Also applies to: 45-46
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/load-credentials.ts` around lines 4 - 24, Remove the long operator-facing systemd/proc rationale block from src/load-credentials.ts and move that content into README or operator docs; in the source file keep a concise JSDoc that states the loader's purpose (bridge credential files into process.env) and the essential behavior points only: it maps credential filenames to process.env vars, only fills blank env vars, and is a no-op when $CREDENTIALS_DIRECTORY is unset. Update the file-level comment near the top of src/load-credentials.ts (and the similar note at lines 45-46) to the terse version and place the detailed operational explanation in documentation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/load-credentials.test.ts`:
- Around line 21-22: The cleanup currently unconditionally deletes
process.env["CREDENTIALS_DIRECTORY"], which can remove a pre-existing value;
change the cleanup to only delete CREDENTIALS_DIRECTORY if it wasn't present in
the savedEnv snapshot (i.e., if savedEnv does not have the CREDENTIALS_DIRECTORY
key), otherwise restore the original value from savedEnv so the environment is
returned to its prior state; refer to the savedEnv variable and
process.env["CREDENTIALS_DIRECTORY"] in your change.
---
Nitpick comments:
In `@src/index.ts`:
- Around line 161-163: Remove the WHAT-style startup behavior comment block and
rely on the clear function name loadSystemdCredentials() at the call site;
delete the explanatory lines describing systemd-delivered secrets and the
CREDENTIALS_DIRECTORY mechanics and leave the call (or a short one-line comment
only if needed for intent like "load systemd credentials") so operational
details remain in external docs rather than inline.
In `@src/load-credentials.ts`:
- Around line 4-24: Remove the long operator-facing systemd/proc rationale block
from src/load-credentials.ts and move that content into README or operator docs;
in the source file keep a concise JSDoc that states the loader's purpose (bridge
credential files into process.env) and the essential behavior points only: it
maps credential filenames to process.env vars, only fills blank env vars, and is
a no-op when $CREDENTIALS_DIRECTORY is unset. Update the file-level comment near
the top of src/load-credentials.ts (and the similar note at lines 45-46) to the
terse version and place the detailed operational explanation in documentation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7e8a5c46-a70b-41c7-bba4-e9c91f2089cf
📒 Files selected for processing (3)
src/index.tssrc/load-credentials.tstest/load-credentials.test.ts
The afterEach loop plus Object.assign(savedEnv) already restores the environment exactly; the extra unconditional delete dropped a CREDENTIALS_DIRECTORY that was set before the suite ran, leaking state into later tests. (CodeRabbit feedback on rohitg00#743)
|
Addressed the cleanup feedback in 558b003: the @coderabbitai review |
|
✅ Actions performedReview triggered.
|
1 participant
Problem
Run agentmemory as a systemd service and the only ways to pass provider keys (
ANTHROPIC_API_KEY,OPENAI_API_KEY,AGENTMEMORY_SECRET, ...) areEnvironment=/EnvironmentFile=- both place the secrets in the process environment, readable via/proc/<pid>/environand plaintext at rest.Change
An opt-in loader,
loadSystemdCredentials(), called once at the start of the workermain(). It copies any files in$CREDENTIALS_DIRECTORYintoprocess.env(only when unset/empty; trailing newline trimmed; file name maps 1:1 to the env var). A value assigned toprocess.envafterexecis readable in-process but is not written back to the exec environ block, so it does not appear in/proc/<pid>/environ(verified on Node 22 / glibc). The providers get the keys; the environment stays clean.Requirements & platform support
Purely additive and a no-op when
$CREDENTIALS_DIRECTORYis unset, so non-systemd hosts (macOS, Windows, Alpine/OpenRC) and existingEnvironment=/.envsetups are entirely unaffected.LoadCredentialEncrypted=+systemd-creds- systemd >= 250 (Debian 12+, RHEL/Rocky/Alma 9+, Fedora, Arch, openSUSE, Ubuntu 24.04+).LoadCredential=- systemd >= 247 (e.g. Ubuntu 22.04).Enabling it (operator, systemd >= 250)
systemd decrypts into a private tmpfs (
$CREDENTIALS_DIRECTORY, mode 0400) at start; the loader reads it from there.Compatibility
Explicitly set env vars always win. Unit tests cover populate/trim, no-overwrite, fill-empty, unset/missing dir, and ignoring subdirs/empty files.
Summary by CodeRabbit
New Features
Tests