Skip to content

Deva/chore docs and bug fixes#212

Merged
lroolle merged 6 commits intomainfrom
deva/chore
Mar 12, 2026
Merged

Deva/chore docs and bug fixes#212
lroolle merged 6 commits intomainfrom
deva/chore

Conversation

@lroolle
Copy link
Member

@lroolle lroolle commented Mar 12, 2026

No description provided.

- Generate en_US.UTF-8 locale to prevent setlocale warnings at startup
- Add retry loop (5 attempts) for npm global installs to handle flaky network
- Verify CLI tools by invoking binaries directly instead of npm list
- Add set -eux for stricter error handling in user npm install step
- Add -Q/--quick flag for bare mode: no config mounts, no .deva, implies --rm
- Backup credentials to tmpdir when auth override active, restore on exit
- Include config hash in container name for explicit --config-home
- Skip ~/.config/deva and ~/.cache/deva when -c is explicit (isolation)
- Warn when explicit config-home has empty auth directories
- Skip filesystem operations during --dry-run
- Redact secrets in debug output
- Add ANTHROPIC_AUTH_TOKEN and ANTHROPIC_BASE_URL env var support
- Replace mv/restore credential backup with overlay file mounting
- Filter sensitive files (.credentials.json, auth.json) from home mounts
- Extract should_mount_home_item() for cleaner mount logic
- formalize the repo with MIT, SECURITY, and CONTRIBUTING docs
- rewrite README and fix installer module drift
- ship auth token forwarding and safer credential overlays
- Add comprehensive docs pages (quick-start, auth, troubleshooting,
  etc.)
- Add GitHub Pages workflow and CI docs build job
- Streamline README by moving detailed content to docs site
Copilot AI review requested due to automatic review settings March 12, 2026 04:20
@claude-yolo
Copy link
Contributor

claude-yolo bot commented Mar 12, 2026

Claude finished @lroolle's task in 2m 2s —— View job


Security Audit Complete

I've performed a thorough security audit of this PR focusing on container boundaries, authentication flows, shell entrypoints, and privilege escalation vectors.

Todo List

  • Analyze main deva.sh script for security vulnerabilities
  • Review Dockerfile changes for container security issues
  • Examine authentication flows in agent scripts
  • Check for shell injection vulnerabilities
  • Assess privilege escalation vectors
  • Review CI/CD workflow security
  • Analyze new documentation for security gaps
  • Compile findings and risk assessment

🔴 BLOCKING SECURITY ISSUES

1. Command Injection in Volume Hash Computation

File: deva.sh lines 632-636, 2230-2234, 2242-2246
Severity: CRITICAL
Issue: Multiple uses of printf piped to hash commands without proper escaping:

config_hash=$(printf '%s' "$config_hash_source" | md5sum | cut -c1-6)
creds_hash=$(printf '%s' "$CUSTOM_CREDENTIALS_FILE" | md5sum | cut -c1-8)

Impact: User-controlled paths in $CUSTOM_CREDENTIALS_FILE or $config_hash_source could inject commands
Fix Required: Use -- to terminate option parsing: printf -- '%s' "$var"

2. Unsafe Command Substitution Pattern

File: deva.sh lines throughout (160, 245, 309, etc.)
Severity: HIGH
Issue: Extensive use of $(...) command substitution without input validation
Examples:

raw="$(expand_tilde "$raw")"
dir="$(pwd)" 
abs_src=$(cd "$(dirname "$src")" 2>/dev/null && pwd)/$(basename "$src")

Impact: If input paths contain command injection chars, could lead to RCE
Mitigation: The existing validate_config_value() function provides some protection but isn't consistently applied

3. Privileged Docker Socket Access by Default

File: deva.sh line ~770 (auto-mount logic)
Severity: HIGH
Issue: Docker socket (/var/run/docker.sock) auto-mounted when present
Impact: Container gains root-equivalent access to host Docker daemon

  • Can start privileged containers
  • Can mount any host path
  • Effective container escape vector
    Recommendation: Require explicit --enable-docker flag instead of --no-docker opt-out

4. Password-less Sudo in Container

File: Dockerfile lines 180-182
Severity: HIGH
Issue: deva user has NOPASSWD sudo access

echo "$DEVA_USER ALL=(ALL) NOPASSWD: ALL" > "/etc/sudoers.d/$DEVA_USER"

Impact: Any command execution as deva user can escalate to root in container
Justification: Documented as "development convenience" but significantly weakens container security model

🟠 HIGH-RISK FINDINGS

5. Unvalidated Environment Variable Processing

File: deva.sh expand_env_value() function
Issue: Environment variable expansion without sufficient validation
Risk: Could allow injection through carefully crafted env vars

6. Authentication Token Exposure Risk

File: agents/shared_auth.sh lines 95, 89
Issue: GitHub tokens passed as command line arguments:

cmd+=(--github-token "${GH_TOKEN:-${GITHUB_TOKEN}}")

Risk: Tokens visible in process list, docker inspect output

7. Insecure Download in Install Script

File: install.sh lines 44-45
Issue: curl without checksum verification:

curl -fsSL "$GITHUB_RAW/$path" -o "$dest"

Risk: MITM attacks, supply chain compromise (though over HTTPS)

8. Unsafe Copilot API Installation

File: Dockerfile lines 94-101
Issue: Git clone and npm install from external repo without integrity verification
Risk: Supply chain attack vector through compromised upstream

🟡 MEDIUM-RISK FINDINGS

9. Weak File Permission Validation

File: deva.sh should_mount_home_item()
Issue: Blacklist approach for credential files
Risk: May miss new credential file patterns

10. Race Condition in Container Selection

File: deva.sh container picking logic
Issue: TOCTOU between container enumeration and selection
Risk: Could attach to wrong container in rare cases

✅ POSITIVE SECURITY IMPROVEMENTS

Good Security Practices Found:

  1. Input validation via validate_config_value() function (lines 1306-1320)
  2. Command substitution restrictions - only allows $(pwd)
  3. Credential file filtering in should_mount_home_item()
  4. Auth environment variable isolation via should_skip_env_for_auth()
  5. Tainted credential detection in has_auth_override()
  6. Security documentation added in SECURITY.md
  7. Set -euo pipefail in install.sh (improved error handling)

REQUIRED TESTS BEFORE MERGE

  1. Shell injection tests:

    # Test injection in custom credentials file path
    deva.sh --auth-with './file$(id).json'
    
    # Test injection in volume paths  
    deva.sh -v './path$(whoami):/target'
  2. Docker socket privilege tests:

    # Verify container cannot escape via docker socket
    deva.sh -- /bin/sh -c 'docker run --privileged --rm alpine id'
  3. Authentication isolation tests:

    # Verify auth tokens don't leak between auth methods
    export ANTHROPIC_API_KEY="test-key" 
    deva.sh claude --auth-with copilot
    # Should not see ANTHROPIC_API_KEY in container

RECOMMENDATIONS

Immediate (Pre-merge):

  1. Fix printf injection - add -- flags to all printf calls with user input
  2. Document Docker socket risk prominently in README
  3. Add integrity checks to install.sh downloads

Short-term:

  1. Change Docker socket default to opt-in
  2. Implement checksum verification for external dependencies
  3. Add process argument sanitization for token passing

Long-term:

  1. Implement proper secrets management instead of env vars
  2. Consider rootless container runtime option
  3. Add security scanning to CI pipeline

Overall Assessment: This PR improves security with better input validation and credential handling, but contains several injection vulnerabilities that must be fixed before merge. The Docker socket auto-mount significantly weakens the "container as sandbox" model.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR prepares the repo for an OSS-style release by adding a MkDocs documentation site, polishing project metadata (README/license/security/contributing), and tightening auth/config handling in the deva.sh launcher (including --dry-run side-effect fixes and container identity changes).

Changes:

  • Add MkDocs configuration, docs pages, and CI/Pages workflows to build and publish the documentation site.
  • Update installer + release workflow docs to reflect deva.sh as the primary entrypoint and to install the full agent set.
  • Refine deva.sh auth/config behavior (quick mode, credential overlays, container naming, dry-run behavior) and extend agent auth support (e.g., ANTHROPIC_AUTH_TOKEN).

Reviewed changes

Copilot reviewed 25 out of 26 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
workflows/RELEASE.md Updates release instructions to use deva.sh as version source.
mkdocs.yml Adds MkDocs Material site config + navigation.
install.sh Refreshes installer for multi-agent + fallback image behavior.
docs/index.md Adds docs landing page.
docs/quick-start.md Adds quick start guide for install + basic usage.
docs/how-it-works.md Documents startup/config/auth/mount/runtime model.
docs/authentication.md Documents per-agent auth modes and mounting behavior.
docs/advanced-usage.md Documents .deva, profiles, risk knobs, advanced workflows.
docs/troubleshooting.md Adds troubleshooting playbook and diagnostic commands.
docs-requirements.txt Pins MkDocs + theme dependencies for CI build.
deva.sh Version bump + quick mode + container identity/auth overlay + dry-run safety improvements.
agents/shared_auth.sh Extends Anthropic token validation to include ANTHROPIC_AUTH_TOKEN.
agents/claude.sh Adds ANTHROPIC_AUTH_TOKEN support and avoids proxy start/model probing during --dry-run.
agents/codex.sh Avoids proxy start/model probing during --dry-run.
agents/gemini.sh Adjusts gemini mounts + settings writes (now gated by DRY_RUN).
README.md Rewrites README into concise landing page + docs links.
CHANGELOG.md Adds 0.9.2 release notes and updates product name.
DEV-LOGS.md Adds dev log entries describing OSS/docs/auth work.
CONTRIBUTING.md Adds contribution workflow + local checks guidance.
SECURITY.md Adds vulnerability reporting policy.
LICENSE Adds MIT license file.
Dockerfile Adds locale generation + npm install retry + CLI version sanity checks.
.gitignore Ignores MkDocs site/ output.
.github/workflows/ci.yml Adds MkDocs build job to CI.
.github/workflows/pages.yml Adds GitHub Pages workflow to build/deploy docs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines +1033 to +1034
mkdir -p "$state_dir"
cp "$claude_json" "$state_dir/.claude.json.bak"
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

backup_claude_json copies .claude.json into $XDG_STATE_HOME/.../deva/backups but doesn’t enforce restrictive permissions on the backup. Since .claude.json can contain auth/session material, the backup should be written with 0600 (and ideally atomically) to avoid accidentally making it more widely readable than the original.

Suggested change
mkdir -p "$state_dir"
cp "$claude_json" "$state_dir/.claude.json.bak"
local backup_path tmp_backup
backup_path="$state_dir/.claude.json.bak"
# Ensure backup directory exists and is owner-only.
mkdir -p "$state_dir"
chmod 700 "$state_dir" 2>/dev/null || true
# Create a temporary file with restrictive permissions, then atomically move.
tmp_backup="$(mktemp "$state_dir/.claude.json.bak.XXXXXX")"
chmod 600 "$tmp_backup" 2>/dev/null || true
if ! cp "$claude_json" "$tmp_backup"; then
rm -f "$tmp_backup"
return 1
fi
mv -f "$tmp_backup" "$backup_path"

Copilot uses AI. Check for mistakes.
Comment on lines +1086 to +1087
fi
DOCKER_ARGS+=("-v" "$overlay_file:$target_path")
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

append_auth_credential_overlay bind-mounts the generated blank overlay file without :ro. Since this file exists on the host under $XDG_STATE_HOME, mounting it read-write allows the container/agent to modify host state. Mount the overlay as read-only (and consider writing it with restrictive perms) so the overlay can’t be mutated from inside the container.

Suggested change
fi
DOCKER_ARGS+=("-v" "$overlay_file:$target_path")
chmod 600 "$overlay_file"
fi
DOCKER_ARGS+=("-v" "$overlay_file:$target_path:ro")

Copilot uses AI. Check for mistakes.
Comment on lines 81 to 100
@@ -86,9 +93,10 @@ setup_gemini_auth() {
}
}
EOF
echo "Created gemini settings with API key auth: $settings_file" >&2
else
echo "Using existing gemini settings: $settings_file" >&2
echo "Created gemini settings with API key auth: $settings_file" >&2
else
echo "Using existing gemini settings: $settings_file" >&2
fi
fi
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In --auth-with api-key for Gemini, this code still writes to a host-side $HOME/.gemini (mkdir/rm/settings.json) when neither CONFIG_HOME nor CONFIG_ROOT is set. In -Q/--quick bare mode, this means a run can unexpectedly mutate the user’s real home config even though bare mode is intended to avoid host config mounts/state. Consider short-circuiting these writes when QUICK_MODE=true (or require an explicit config home) to keep -Q side-effect-free on the host.

Copilot uses AI. Check for mistakes.
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a69ce81b86

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +2316 to +2319
for d in "$CONFIG_ROOT"/*; do
[ -d "$d" ] || continue
[ "$(basename "$d")" = "_shared" ] && continue
mount_dir_contents_into_home "$d"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restrict non-active credential mounts in config-root mode

When --config-home is a root containing multiple agent folders, this loop mounts every agent directory into the container even under non-default auth (for example, claude --auth-with api-key). The new overlay logic only blanks the active agent credential path, so inactive credentials like /home/deva/.codex/auth.json remain mounted and writable, reintroducing credential exposure/corruption risk in the mode that is supposed to isolate from OAuth files.

Useful? React with 👍 / 👎.

Comment on lines 78 to +81
gemini_config_dir="$HOME/.gemini"
fi

mkdir -p "$gemini_config_dir"
rm -f "$gemini_config_dir/mcp-oauth-tokens-v2.json"
if [ "${DRY_RUN:-false}" != true ]; then

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Skip host Gemini config writes in quick API-key mode

In -Q/--quick runs, deva.sh clears CONFIG_HOME/CONFIG_ROOT, so this fallback resolves to $HOME/.gemini and then writes host config (mkdir, token cleanup, settings.json) for API-key auth. That breaks quick mode’s bare/no-config expectation and the written settings are not mounted into the container anyway, so the auth-mode preparation can be ineffective for the actual run while still mutating host state.

Useful? React with 👍 / 👎.

- publish docs with docs.deva.sh domain metadata and site links
- share tool version resolution across nightly and tagged releases
@claude-yolo
Copy link
Contributor

claude-yolo bot commented Mar 12, 2026

Claude finished @lroolle's task in 2m 50s —— View job


Security Audit Complete

I've performed a thorough security audit of this PR focusing on container boundaries, authentication flows, shell entrypoints, and privilege escalation vectors.

Todo List

  • Analyze main deva.sh script for security vulnerabilities
  • Review Dockerfile changes for container security issues
  • Examine authentication flows in agent scripts
  • Check for shell injection vulnerabilities
  • Assess privilege escalation vectors
  • Review CI/CD workflow security
  • Analyze new documentation for security gaps
  • Compile findings and risk assessment

🔴 BLOCKING SECURITY ISSUES

1. Command Injection in Volume Hash Computation

File: deva.sh lines 632-636, 2230-2234, 2242-2246
Severity: CRITICAL
Issue: Multiple uses of printf piped to hash commands without proper escaping:

config_hash=$(printf '%s' "$config_hash_source" | md5sum | cut -c1-6)
creds_hash=$(printf '%s' "$CUSTOM_CREDENTIALS_FILE" | md5sum | cut -c1-8)

Impact: User-controlled paths in $CUSTOM_CREDENTIALS_FILE or $config_hash_source could inject commands
Fix Required: Use -- to terminate option parsing: printf -- '%s' "$var"

2. Unsafe Command Substitution Pattern

File: deva.sh lines throughout (160, 245, 309, etc.)
Severity: HIGH
Issue: Extensive use of $(...) command substitution without input validation
Examples:

raw="$(expand_tilde "$raw")"
dir="$(pwd)" 
abs_src=$(cd "$(dirname "$src")" 2>/dev/null && pwd)/$(basename "$src")

Impact: If input paths contain command injection chars, could lead to RCE
Mitigation: The existing validate_config_value() function provides some protection but isn't consistently applied

3. Privileged Docker Socket Access by Default

File: deva.sh line ~770 (auto-mount logic)
Severity: HIGH
Issue: Docker socket (/var/run/docker.sock) auto-mounted when present
Impact: Container gains root-equivalent access to host Docker daemon

  • Can start privileged containers
  • Can mount any host path
  • Effective container escape vector
    Recommendation: Require explicit --enable-docker flag instead of --no-docker opt-out

4. Password-less Sudo in Container

File: Dockerfile lines 180-182
Severity: HIGH
Issue: deva user has NOPASSWD sudo access

echo "$DEVA_USER ALL=(ALL) NOPASSWD: ALL" > "/etc/sudoers.d/$DEVA_USER"

Impact: Any command execution as deva user can escalate to root in container
Justification: Documented as "development convenience" but significantly weakens container security model

🟠 HIGH-RISK FINDINGS

5. Unvalidated Environment Variable Processing

File: deva.sh expand_env_value() function
Issue: Environment variable expansion without sufficient validation
Risk: Could allow injection through carefully crafted env vars

6. Authentication Token Exposure Risk

File: agents/shared_auth.sh lines 95, 89
Issue: GitHub tokens passed as command line arguments:

cmd+=(--github-token "${GH_TOKEN:-${GITHUB_TOKEN}}")

Risk: Tokens visible in process list, docker inspect output

7. Insecure Download in Install Script

File: install.sh lines 44-45
Issue: curl without checksum verification:

curl -fsSL "$GITHUB_RAW/$path" -o "$dest"

Risk: MITM attacks, supply chain compromise (though over HTTPS)

8. Unsafe Copilot API Installation

File: Dockerfile lines 94-101
Issue: Git clone and npm install from external repo without integrity verification
Risk: Supply chain attack vector through compromised upstream

🟡 MEDIUM-RISK FINDINGS

9. Weak File Permission Validation

File: deva.sh should_mount_home_item()
Issue: Blacklist approach for credential files
Risk: May miss new credential file patterns

10. Race Condition in Container Selection

File: deva.sh container picking logic
Issue: TOCTOU between container enumeration and selection
Risk: Could attach to wrong container in rare cases

✅ POSITIVE SECURITY IMPROVEMENTS

Good Security Practices Found:

  1. Input validation via validate_config_value() function (lines 1306-1320)
  2. Command substitution restrictions - only allows $(pwd)
  3. Credential file filtering in should_mount_home_item()
  4. Auth environment variable isolation via should_skip_env_for_auth()
  5. Tainted credential detection in has_auth_override()
  6. Security documentation added in SECURITY.md
  7. Set -euo pipefail in install.sh (improved error handling)

REQUIRED TESTS BEFORE MERGE

  1. Shell injection tests:

    # Test injection in custom credentials file path
    deva.sh --auth-with './file$(id).json'
    
    # Test injection in volume paths  
    deva.sh -v './path$(whoami):/target'
  2. Docker socket privilege tests:

    # Verify container cannot escape via docker socket
    deva.sh -- /bin/sh -c 'docker run --privileged --rm alpine id'
  3. Authentication isolation tests:

    # Verify auth tokens don't leak between auth methods
    export ANTHROPIC_API_KEY="test-key" 
    deva.sh claude --auth-with copilot
    # Should not see ANTHROPIC_API_KEY in container

RECOMMENDATIONS

Immediate (Pre-merge):

  1. Fix printf injection - add -- flags to all printf calls with user input
  2. Document Docker socket risk prominently in README
  3. Add integrity checks to install.sh downloads

Short-term:

  1. Change Docker socket default to opt-in
  2. Implement checksum verification for external dependencies
  3. Add process argument sanitization for token passing

Long-term:

  1. Implement proper secrets management instead of env vars
  2. Consider rootless container runtime option
  3. Add security scanning to CI pipeline

Overall Assessment: This PR improves security with better input validation and credential handling, but contains several injection vulnerabilities that must be fixed before merge. The Docker socket auto-mount significantly weakens the "container as sandbox" model.

@lroolle lroolle merged commit f8f8591 into main Mar 12, 2026
4 checks passed
@lroolle lroolle deleted the deva/chore branch March 12, 2026 04:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants