feat: CA certificate support for corporate SSL inspection - #62
Conversation
Adds CA_CERT config variable (.dclaude) and DCLAUDE_CA_CERT env var to configure a custom CA certificate for environments with corporate SSL inspection proxies. Sets NODE_EXTRA_CA_CERTS and SSL_CERT_FILE inside the container, resolves relative paths, and validates the cert is within the mounted directory tree. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
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 skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds support for a corporate CA certificate via a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ConfigFile
participant dclaude
participant FS as FileSystem
participant Docker
User->>dclaude: start (env + optional .dclaude)
dclaude->>ConfigFile: load_config_file()
ConfigFile-->>dclaude: CA_CERT (relative or abs) or unset
dclaude->>FS: resolve relative CA_CERT (realpath vs PWD fallback)
FS-->>dclaude: absolute_path or missing
dclaude->>dclaude: validate existence and MOUNT_ROOT containment
alt valid and within MOUNT_ROOT
dclaude->>Docker: add env NODE_EXTRA_CA_CERTS, SSL_CERT_FILE
Docker-->>dclaude: container args assembled
else missing or outside MOUNT_ROOT
dclaude-->>User: emit warning (no container env added)
end
dclaude->>Docker: run container (with or without cert env)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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.
🧹 Nitpick comments (2)
dclaude (2)
170-174: Silent failure whenrealpathfails may confuse users.If the user provides a relative path that doesn't exist (e.g.,
DCLAUDE_CA_CERT=./missing.pem),realpathfails andDCLAUDE_CA_CERTbecomes empty due to|| true. The later validation at line 1938 checks[[ ! -f "$CA_CERT" ]], but an emptyCA_CERTpasses this check silently (since[[ -n "$CA_CERT" ]]is the outer condition).Consider preserving the original path so the "not found" warning is more helpful:
🔧 Suggested improvement
# Resolve CA_CERT relative paths (env var: relative to PWD, config file: already resolved) if [[ -n "${DCLAUDE_CA_CERT:-}" && "${DCLAUDE_CA_CERT}" != /* ]]; then - DCLAUDE_CA_CERT=$(realpath "$DCLAUDE_CA_CERT" 2>/dev/null) || true + DCLAUDE_CA_CERT=$(realpath "$DCLAUDE_CA_CERT" 2>/dev/null) || DCLAUDE_CA_CERT="$PWD/$DCLAUDE_CA_CERT" fiThis preserves the intended path for the "not found" warning rather than clearing it entirely.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dclaude` around lines 170 - 174, The current block uses realpath on DCLAUDE_CA_CERT but swallows failures with "|| true", which clears the variable and hides the original path; change the logic so that realpath output is only assigned to DCLAUDE_CA_CERT when realpath succeeds and otherwise leave DCLAUDE_CA_CERT unchanged (preserving the user-provided path) so the later CA_CERT existence check ([[ ! -f "$CA_CERT" ]]) can emit a helpful "not found" warning; specifically, call realpath and check its exit status (or capture its output into a temp var and assign only on success) when resolving DCLAUDE_CA_CERT before exporting readonly CA_CERT.
1935-1950: Path prefix check has a subtle edge case.The check
[[ "$CA_CERT" != "$MOUNT_ROOT"* ]]correctly validates that the cert is under the mount root. However, it would incorrectly pass for paths like/home/user-other/cert.pemwhenMOUNT_ROOT=/home/user(since the glob matches the prefix).A more robust check would ensure a path separator:
🛡️ More precise path containment check
- elif [[ "$CA_CERT" != "$MOUNT_ROOT"* ]]; then + elif [[ "$CA_CERT" != "$MOUNT_ROOT"/* && "$CA_CERT" != "$MOUNT_ROOT" ]]; thenThis ensures the certificate path is either exactly the mount root (unlikely for a file) or is under it with a path separator. That said, the current implementation is low-risk since accidental matches would require very specific naming coincidences.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dclaude` around lines 1935 - 1950, The prefix check for CA_CERT vs MOUNT_ROOT can false-positive (e.g., /home/user-other when MOUNT_ROOT=/home/user); update the containment logic in the CA certificate handling block to canonicalize both paths (use realpath -m or readlink -f fallback) and then check that canonical CA_CERT is equal to the canonical MOUNT_ROOT or starts with the canonical MOUNT_ROOT plus a trailing slash; adjust the condition around the CA_CERT/MOUNT_ROOT check (the block that currently uses [[ "$CA_CERT" != "$MOUNT_ROOT"* ]]) and keep adding NODE_EXTRA_CA_CERTS/SSL_CERT_FILE to DOCKER_ARGS only when the containment check passes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@dclaude`:
- Around line 170-174: The current block uses realpath on DCLAUDE_CA_CERT but
swallows failures with "|| true", which clears the variable and hides the
original path; change the logic so that realpath output is only assigned to
DCLAUDE_CA_CERT when realpath succeeds and otherwise leave DCLAUDE_CA_CERT
unchanged (preserving the user-provided path) so the later CA_CERT existence
check ([[ ! -f "$CA_CERT" ]]) can emit a helpful "not found" warning;
specifically, call realpath and check its exit status (or capture its output
into a temp var and assign only on success) when resolving DCLAUDE_CA_CERT
before exporting readonly CA_CERT.
- Around line 1935-1950: The prefix check for CA_CERT vs MOUNT_ROOT can
false-positive (e.g., /home/user-other when MOUNT_ROOT=/home/user); update the
containment logic in the CA certificate handling block to canonicalize both
paths (use realpath -m or readlink -f fallback) and then check that canonical
CA_CERT is equal to the canonical MOUNT_ROOT or starts with the canonical
MOUNT_ROOT plus a trailing slash; adjust the condition around the
CA_CERT/MOUNT_ROOT check (the block that currently uses [[ "$CA_CERT" !=
"$MOUNT_ROOT"* ]]) and keep adding NODE_EXTRA_CA_CERTS/SSL_CERT_FILE to
DOCKER_ARGS only when the containment check passes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c9226d1c-f1d0-40e8-8ecd-c343dfb7ecd9
📒 Files selected for processing (3)
CLAUDE.mdREADME.mddclaude
Ensures the user-provided path is kept for the "not found" warning instead of silently clearing the variable. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 participant
Summary
CA_CERTconfig variable (.dclaudefile) andDCLAUDE_CA_CERTenv var for corporate environments with SSL inspection proxiesNODE_EXTRA_CA_CERTSandSSL_CERT_FILEinside the container so Claude Code, npm, curl, and Python all trust the corporate CAUsage
# .dclaude config file CA_CERT=multitude-ca.pemor
Test plan
DCLAUDE_CA_CERT=./some-cert.pem dclaudesetsNODE_EXTRA_CA_CERTSandSSL_CERT_FILEinside containerCA_CERT=cert.pemin.dclaudeconfig resolves relative to config file directory🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Behavior