Skip to content

Fix installer smoke registry dependency#214

Merged
lroolle merged 2 commits intomainfrom
fix/installer-smoke
Mar 12, 2026
Merged

Fix installer smoke registry dependency#214
lroolle merged 2 commits intomainfrom
fix/installer-smoke

Conversation

@lroolle
Copy link
Member

@lroolle lroolle commented Mar 12, 2026

Summary

  • let install.sh reuse a local image instead of always pulling
  • make installer smoke build its own local image in CI
  • keep the 0.9.2 changelog aligned with the release blocker fix

Verification

  • shellcheck ./install.sh ./deva.sh ./scripts/resolve-tool-versions.sh
  • ./scripts/version-check.sh
  • DEVA_INSTALL_BASE_URL=file://$PWD DEVA_DOCKER_IMAGE=deva-smoke:test bash ./install.sh

Context

  • ghcr.io/thevibeworks/deva is currently private, so the old smoke job and public installer path were both depending on registry auth

- reuse a prebuilt local image instead of blindly pulling every time
- make installer smoke build its own image so CI stays hermetic
- keep 0.9.2 release notes aligned with the actual fix
Copilot AI review requested due to automatic review settings March 12, 2026 06:21
@claude-yolo
Copy link
Contributor

claude-yolo bot commented Mar 12, 2026

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


Security Audit Complete

Performed paranoid security audit of installer smoke registry dependency fix.

Todo List

  • Read and analyze current install.sh for baseline security posture
  • Examine CI workflow changes for privilege escalation risks
  • Review PR diff for shell injection vulnerabilities
  • Audit container image handling and registry authentication bypasses
  • Check for input validation and sanitization issues
  • Identify blocking security issues and required tests
  • Provide comprehensive security review with specific findings

Security Audit Results

SAFE CHANGES - No Blocking Issues Found

Shell Injection Analysis:

  • ✅ All docker image variables properly quoted: "$DOCKER_IMAGE", "${DEVA_DOCKER_IMAGE}:${DEVA_DOCKER_TAG}"
  • ✅ Environment variables used safely in docker commands (lines 2441, 2484 in deva.sh)
  • ✅ install.sh follows secure quoting patterns throughout

Container Image Handling:

  • ✅ New logic safely checks for local images first: docker image inspect "$DOCKER_IMAGE" >/dev/null 2>&1
  • ✅ Fallback logic properly handles registry failures with error checking
  • ✅ CI now builds local images instead of depending on private registry auth

🔍 Security Observations

Environment Variable Injection Risk (LOW):

  • DEVA_DOCKER_IMAGE/DEVA_DOCKER_IMAGE_FALLBACK accept arbitrary values without validation
  • Mitigation: Values are properly quoted in all docker commands, preventing shell injection
  • Risk Assessment: LOW - requires host-level compromise to exploit

Registry Authentication Bypass (INTENTIONAL):

  • PR correctly removes docker/login-action from CI smoke tests
  • Context: ghcr.io/thevibeworks/deva is private, causing CI failures
  • Mitigation: CI now builds local deva-smoke:ci image for testing

📋 Required Security Tests

Pre-merge validation needed:

# Test environment variable handling
export DEVA_DOCKER_IMAGE="malicious;rm -rf /" && ./install.sh  # Should fail safely
export DEVA_DOCKER_IMAGE="$(echo evil)" && ./install.sh        # Should fail safely  
export DEVA_DOCKER_IMAGE="" && ./install.sh                     # Should use default

# Test CI smoke build
docker build -t deva-smoke:ci .
DEVA_DOCKER_IMAGE="deva-smoke:ci" bash ./install.sh

🔐 Security Boundaries Validated

Container Security Model:

  • ✅ Container remains the security boundary (per CLAUDE.md)
  • ✅ Host bridges unchanged: /var/run/docker.sock, auth mounts, tmux bridge
  • ✅ No new privilege escalation vectors introduced
  • ✅ Agent sandbox behavior unchanged (--dangerously-skip-permissions still used)

Authentication Flow:

  • ✅ Auth model unchanged - only affects CI registry dependency
  • ✅ No impact on user auth workflows or container auth mounts
  • ✅ Private registry access properly isolated to local CI builds

Recommendation: APPROVE

Changes are security-conscious and properly implement the intended registry independence. The container security model remains intact, and no new attack vectors are introduced.

- build the local smoke image with the same version inputs as release
- stop failing on unset Dockerfile build args before install can run
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 removes the installer smoke test’s dependency on private registry auth by allowing install.sh to reuse a prebuilt local Docker image and updating CI to build a local image for the smoke job.

Changes:

  • install.sh now checks for an existing local image before pulling, and adds configurable primary/fallback image env vars.
  • CI smoke job now builds a local Docker image instead of logging into GHCR.
  • Changelog entry added for the installer/smoke registry-auth fix.

Reviewed changes

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

File Description
install.sh Adds local-image reuse and configurable pull/fallback behavior.
CHANGELOG.md Documents the installer/smoke fix for 0.9.2.
.github/workflows/ci.yml Builds a local smoke image and attempts to point the smoke run at it.

💡 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.

DOCKER_IMAGE="ghcr.io/thevibeworks/deva:latest"
DOCKER_IMAGE_FALLBACK="thevibeworks/deva:latest"
DOCKER_IMAGE="${DEVA_DOCKER_IMAGE:-ghcr.io/thevibeworks/deva:latest}"
DOCKER_IMAGE_FALLBACK="${DEVA_DOCKER_IMAGE_FALLBACK:-thevibeworks/deva:latest}"
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_IMAGE_FALLBACK uses ${DEVA_DOCKER_IMAGE_FALLBACK:-...}, which treats an explicitly empty DEVA_DOCKER_IMAGE_FALLBACK as “unset” and re-enables the default fallback. That makes it impossible to disable the fallback pull (e.g., CI exports DEVA_DOCKER_IMAGE_FALLBACK="" but the script still falls back). Use the non-: form to honor empty string, or add explicit logic to interpret empty as “no fallback”.

Suggested change
DOCKER_IMAGE_FALLBACK="${DEVA_DOCKER_IMAGE_FALLBACK:-thevibeworks/deva:latest}"
DOCKER_IMAGE_FALLBACK="${DEVA_DOCKER_IMAGE_FALLBACK-thevibeworks/deva:latest}"

Copilot uses AI. Check for mistakes.
echo "Primary pull failed. Trying fallback image..."
docker pull "$DOCKER_IMAGE_FALLBACK"
echo ""
echo "warning: using fallback image $DOCKER_IMAGE_FALLBACK"
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.

After successfully pulling the fallback image, the installer only prints a warning but doesn’t give an actionable way to run deva.sh with that fallback. Since deva.sh defaults to ghcr.io/thevibeworks/deva:latest, installs in environments without GHCR access will still fail at first launch unless the user sets the image env vars. Consider printing the exact export DEVA_DOCKER_IMAGE=.../DEVA_DOCKER_TAG=... guidance (or switching the effective image for the remainder of the session).

Suggested change
echo "warning: using fallback image $DOCKER_IMAGE_FALLBACK"
echo "warning: using fallback image $DOCKER_IMAGE_FALLBACK"
echo "note: deva.sh defaults to $DOCKER_IMAGE and may fail without access to that registry."
echo " To use the fallback image in this shell, run:"
echo " export DEVA_DOCKER_IMAGE=\"$DOCKER_IMAGE_FALLBACK\""
echo " You can add that line to your shell profile to make the change permanent."

Copilot uses AI. Check for mistakes.
export HOME="$(mktemp -d)"
export PATH="$HOME/.local/bin:$PATH"
export DEVA_INSTALL_BASE_URL="file://$PWD"
export DEVA_DOCKER_IMAGE="deva-smoke:ci"
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.

DEVA_DOCKER_IMAGE in deva.sh is a repo name and deva.sh appends :${DEVA_DOCKER_TAG}. Setting DEVA_DOCKER_IMAGE="deva-smoke:ci" here will make deva.sh try to run deva-smoke:ci:latest (invalid image reference) and break the smoke test. Set DEVA_DOCKER_IMAGE to deva-smoke and DEVA_DOCKER_TAG to ci (or adjust deva.sh/installer semantics to accept a fully-qualified image reference).

Suggested change
export DEVA_DOCKER_IMAGE="deva-smoke:ci"
export DEVA_DOCKER_IMAGE="deva-smoke"
export DEVA_DOCKER_TAG="ci"

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: 78ba92bf70

ℹ️ 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".

export HOME="$(mktemp -d)"
export PATH="$HOME/.local/bin:$PATH"
export DEVA_INSTALL_BASE_URL="file://$PWD"
export DEVA_DOCKER_IMAGE="deva-smoke:ci"

Choose a reason for hiding this comment

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

P1 Badge Split smoke image repo and tag env vars

This step sets DEVA_DOCKER_IMAGE=deva-smoke:ci, but deva.sh later always composes image references as ${DEVA_DOCKER_IMAGE}:${DEVA_DOCKER_TAG} (for both check_image and docker run), so the smoke commands resolve to an invalid/different reference like deva-smoke:ci:latest instead of the locally built deva-smoke:ci. In the CI smoke context, that makes deva.sh ... --version fail before the intended non-TTY launch check; set repo/tag separately (DEVA_DOCKER_IMAGE=deva-smoke, DEVA_DOCKER_TAG=ci) or build/tag :latest.

Useful? React with 👍 / 👎.

DOCKER_IMAGE="ghcr.io/thevibeworks/deva:latest"
DOCKER_IMAGE_FALLBACK="thevibeworks/deva:latest"
DOCKER_IMAGE="${DEVA_DOCKER_IMAGE:-ghcr.io/thevibeworks/deva:latest}"
DOCKER_IMAGE_FALLBACK="${DEVA_DOCKER_IMAGE_FALLBACK:-thevibeworks/deva:latest}"

Choose a reason for hiding this comment

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

P2 Badge Preserve empty fallback override in installer

Using ${DEVA_DOCKER_IMAGE_FALLBACK:-thevibeworks/deva:latest} treats an explicitly empty value as unset, so DEVA_DOCKER_IMAGE_FALLBACK="" cannot actually disable fallback pulls even though the later branch checks for an empty string. In environments where the primary pull fails, this silently reintroduces a Docker Hub dependency and can break offline/private-registry install flows that this change is trying to decouple.

Useful? React with 👍 / 👎.

@claude-yolo
Copy link
Contributor

claude-yolo bot commented Mar 12, 2026

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


Security Audit Complete

Performed paranoid security audit of installer smoke registry dependency fix.

Todo List

  • Read and analyze current install.sh for baseline security posture
  • Examine CI workflow changes for privilege escalation risks
  • Review PR diff for shell injection vulnerabilities
  • Audit container image handling and registry authentication bypasses
  • Check for input validation and sanitization issues
  • Identify blocking security issues and required tests
  • Provide comprehensive security review with specific findings

Security Audit Results

SAFE CHANGES - No Blocking Issues Found

Shell Injection Analysis:

  • ✅ All docker image variables properly quoted: "$DOCKER_IMAGE", "${DEVA_DOCKER_IMAGE}:${DEVA_DOCKER_TAG}"
  • ✅ Environment variables used safely in docker commands (lines 2441, 2484 in deva.sh)
  • ✅ install.sh follows secure quoting patterns throughout

Container Image Handling:

  • ✅ New logic safely checks for local images first: docker image inspect "$DOCKER_IMAGE" >/dev/null 2>&1
  • ✅ Fallback logic properly handles registry failures with error checking
  • ✅ CI now builds local images instead of depending on private registry auth

🔍 Security Observations

Environment Variable Injection Risk (LOW):

  • DEVA_DOCKER_IMAGE/DEVA_DOCKER_IMAGE_FALLBACK accept arbitrary values without validation
  • Mitigation: Values are properly quoted in all docker commands, preventing shell injection
  • Risk Assessment: LOW - requires host-level compromise to exploit

Registry Authentication Bypass (INTENTIONAL):

  • PR correctly removes docker/login-action from CI smoke tests
  • Context: ghcr.io/thevibeworks/deva is private, causing CI failures
  • Mitigation: CI now builds local deva-smoke:ci image for testing

📋 Required Security Tests

Pre-merge validation needed:

# Test environment variable handling
export DEVA_DOCKER_IMAGE="malicious;rm -rf /" && ./install.sh  # Should fail safely
export DEVA_DOCKER_IMAGE="$(echo evil)" && ./install.sh        # Should fail safely  
export DEVA_DOCKER_IMAGE="" && ./install.sh                     # Should use default

# Test CI smoke build
docker build -t deva-smoke:ci .
DEVA_DOCKER_IMAGE="deva-smoke:ci" bash ./install.sh

🔐 Security Boundaries Validated

Container Security Model:

  • ✅ Container remains the security boundary (per CLAUDE.md)
  • ✅ Host bridges unchanged: /var/run/docker.sock, auth mounts, tmux bridge
  • ✅ No new privilege escalation vectors introduced
  • ✅ Agent sandbox behavior unchanged (--dangerously-skip-permissions still used)

Authentication Flow:

  • ✅ Auth model unchanged - only affects CI registry dependency
  • ✅ No impact on user auth workflows or container auth mounts
  • ✅ Private registry access properly isolated to local CI builds

Recommendation: APPROVE

Changes are security-conscious and properly implement the intended registry independence. The container security model remains intact, and no new attack vectors are introduced.

@lroolle lroolle merged commit 9afd7dc into main Mar 12, 2026
4 of 5 checks passed
@lroolle lroolle deleted the fix/installer-smoke branch March 12, 2026 06:27
@lroolle lroolle restored the fix/installer-smoke branch March 12, 2026 06:38
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