-
Notifications
You must be signed in to change notification settings - Fork 32
fix: only hide credential files if parent directory exists #737
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -502,8 +502,9 @@ export function generateDockerCompose( | |||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Mount ~/.cargo for Rust binaries (read-only) if it exists | ||||||||||||||||||||||||||
| // SKIP if allowFullFilesystemAccess is false (credentials will be hidden via tmpfs) | ||||||||||||||||||||||||||
| const hostCargoDir = path.join(userHome, '.cargo'); | ||||||||||||||||||||||||||
| if (fs.existsSync(hostCargoDir)) { | ||||||||||||||||||||||||||
| if (fs.existsSync(hostCargoDir) && config.allowFullFilesystemAccess) { | ||||||||||||||||||||||||||
| agentVolumes.push(`${hostCargoDir}:/host${hostCargoDir}:ro`); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
@@ -672,6 +673,9 @@ export function generateDockerCompose( | |||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Store credential tmpfs mounts to add later | ||||||||||||||||||||||||||
| const credentialTmpfsMounts: string[] = []; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Apply security policy: selective mounting vs full filesystem access | ||||||||||||||||||||||||||
| if (config.allowFullFilesystemAccess) { | ||||||||||||||||||||||||||
| // User explicitly opted into full filesystem access - log security warning | ||||||||||||||||||||||||||
|
|
@@ -687,64 +691,66 @@ export function generateDockerCompose( | |||||||||||||||||||||||||
| // This provides security against credential exfiltration via prompt injection | ||||||||||||||||||||||||||
| logger.debug('Using selective mounting for security (credential files hidden)'); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // SECURITY: Hide credential files by mounting /dev/null over them | ||||||||||||||||||||||||||
| // SECURITY: Hide credential directories using tmpfs (empty in-memory filesystem) | ||||||||||||||||||||||||||
| // This prevents prompt-injected commands from reading sensitive tokens | ||||||||||||||||||||||||||
| // even if the attacker knows the file paths | ||||||||||||||||||||||||||
| const credentialFiles = [ | ||||||||||||||||||||||||||
| `${effectiveHome}/.docker/config.json`, // Docker Hub tokens | ||||||||||||||||||||||||||
| `${effectiveHome}/.npmrc`, // NPM registry tokens | ||||||||||||||||||||||||||
| `${effectiveHome}/.cargo/credentials`, // Rust crates.io tokens | ||||||||||||||||||||||||||
| `${effectiveHome}/.composer/auth.json`, // PHP Composer tokens | ||||||||||||||||||||||||||
| `${effectiveHome}/.config/gh/hosts.yml`, // GitHub CLI OAuth tokens | ||||||||||||||||||||||||||
| // SSH private keys (CRITICAL - server access, git operations) | ||||||||||||||||||||||||||
| `${effectiveHome}/.ssh/id_rsa`, | ||||||||||||||||||||||||||
| `${effectiveHome}/.ssh/id_ed25519`, | ||||||||||||||||||||||||||
| `${effectiveHome}/.ssh/id_ecdsa`, | ||||||||||||||||||||||||||
| `${effectiveHome}/.ssh/id_dsa`, | ||||||||||||||||||||||||||
| // Cloud provider credentials (CRITICAL - infrastructure access) | ||||||||||||||||||||||||||
| `${effectiveHome}/.aws/credentials`, | ||||||||||||||||||||||||||
| `${effectiveHome}/.aws/config`, | ||||||||||||||||||||||||||
| `${effectiveHome}/.kube/config`, | ||||||||||||||||||||||||||
| `${effectiveHome}/.azure/credentials`, | ||||||||||||||||||||||||||
| `${effectiveHome}/.config/gcloud/credentials.db`, | ||||||||||||||||||||||||||
| // even if the attacker knows the file paths. | ||||||||||||||||||||||||||
| // Using tmpfs instead of /dev/null mounts avoids Docker errors when parent directories | ||||||||||||||||||||||||||
| // don't exist in the container filesystem. | ||||||||||||||||||||||||||
| const credentialDirs = [ | ||||||||||||||||||||||||||
| `${effectiveHome}/.docker`, // Docker Hub tokens (config.json) | ||||||||||||||||||||||||||
| `${effectiveHome}/.ssh`, // SSH private keys (CRITICAL - server access, git operations) | ||||||||||||||||||||||||||
| `${effectiveHome}/.aws`, // AWS credentials (CRITICAL - infrastructure access) | ||||||||||||||||||||||||||
| `${effectiveHome}/.kube`, // Kubernetes config (CRITICAL - cluster access) | ||||||||||||||||||||||||||
| `${effectiveHome}/.azure`, // Azure credentials | ||||||||||||||||||||||||||
| `${effectiveHome}/.config/gcloud`, // Google Cloud credentials | ||||||||||||||||||||||||||
| `${effectiveHome}/.config/gh`, // GitHub CLI OAuth tokens | ||||||||||||||||||||||||||
| `${effectiveHome}/.cargo`, // Rust crates.io tokens (credentials file) | ||||||||||||||||||||||||||
| `${effectiveHome}/.composer`, // PHP Composer tokens (auth.json) | ||||||||||||||||||||||||||
| ]; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| credentialFiles.forEach(credFile => { | ||||||||||||||||||||||||||
| agentVolumes.push(`/dev/null:${credFile}:ro`); | ||||||||||||||||||||||||||
| // Add tmpfs mounts for credential directories | ||||||||||||||||||||||||||
| credentialDirs.forEach(credDir => { | ||||||||||||||||||||||||||
| credentialTmpfsMounts.push(`${credDir}:rw,noexec,nosuid,size=1m`); | ||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| logger.debug(`Hidden ${credentialFiles.length} credential file(s) via /dev/null mounts`); | ||||||||||||||||||||||||||
| // Also hide ~/.npmrc file (NPM registry tokens) - needs special handling as it's a file | ||||||||||||||||||||||||||
| // Mount its parent directory as tmpfs to hide it | ||||||||||||||||||||||||||
| const npmrcParent = effectiveHome; | ||||||||||||||||||||||||||
| if (!credentialTmpfsMounts.some(mount => mount.startsWith(`${npmrcParent}:`))) { | ||||||||||||||||||||||||||
| // Only add if we're not already mounting the entire home directory | ||||||||||||||||||||||||||
| // In practice, we'll mount ~/.npmrc as a tmpfs (which will be an empty directory) | ||||||||||||||||||||||||||
| credentialTmpfsMounts.push(`${effectiveHome}/.npmrc:rw,noexec,nosuid,size=1m`); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
Comment on lines
+716
to
+723
|
||||||||||||||||||||||||||
| // Also hide ~/.npmrc file (NPM registry tokens) - needs special handling as it's a file | |
| // Mount its parent directory as tmpfs to hide it | |
| const npmrcParent = effectiveHome; | |
| if (!credentialTmpfsMounts.some(mount => mount.startsWith(`${npmrcParent}:`))) { | |
| // Only add if we're not already mounting the entire home directory | |
| // In practice, we'll mount ~/.npmrc as a tmpfs (which will be an empty directory) | |
| credentialTmpfsMounts.push(`${effectiveHome}/.npmrc:rw,noexec,nosuid,size=1m`); | |
| } | |
| // NOTE: ~/.npmrc is typically a file, not a directory. Avoid adding it to the tmpfs | |
| // directory mounts here to prevent Docker "not a directory" errors when it exists | |
| // as a file in the image or home directory. File-level hiding (e.g. bind-mounting | |
| // /dev/null over ~/.npmrc) should be implemented via a separate bind-mount mechanism. |
Copilot
AI
Feb 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const tmpfsMounts = [] is inferred as any[] under TypeScript, which weakens type-safety in a strict project. Declare this as string[] so accidental non-string entries (or later refactors) are caught at compile time.
| const tmpfsMounts = []; | |
| const tmpfsMounts: string[] = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR description/title talk about conditionally adding
/dev/nullmounts based on parent directory existence and logging skipped paths, but the implementation here switches to tmpfs-based directory hiding and doesn’t perform existence checks or log skipped credential paths. Either update the PR description/title to match the new approach or adjust the code to implement the described parent-dir checks + skip logging.