Skip to content

feat: add custom config directory and environment variable support#47

Merged
lroolle merged 7 commits intomainfrom
refactor/envs
Jul 9, 2025
Merged

feat: add custom config directory and environment variable support#47
lroolle merged 7 commits intomainfrom
refactor/envs

Conversation

@lroolle
Copy link
Copy Markdown
Member

@lroolle lroolle commented Jul 8, 2025

Summary

  • Add --config/-c flag for custom config directory isolation
  • Add -e flag for environment variable passing to containers
  • Standardize paths from /root to /home/claude for consistency
  • Fix npm-global path handling for claude user
  • Improve auth isolation between different auth modes

Changes

  • claude-yolo: Added --config/-c and -e flags with help docs
  • claude.sh: Major refactor for custom config and env var support
  • Dockerfile: Fixed npm path and user permission handling
  • docker-entrypoint.sh: Updated user handling for path consistency
  • .gitignore: Added claude-yolo-pro/ exclusion

Test Plan

  • Test --config flag with custom directory
  • Test -e flag with environment variables
  • Verify auth isolation works across all modes
  • Test npm global package installation in container
  • Verify path consistency for all mounts

Closes #46, #31, #32, #24, #38

lroolle added 4 commits July 7, 2025 23:08
- auth: unified --auth-with pattern, proper model handling
- env: add -e flag support and custom config dir
- docker: move to /home/claude, fix permissions
- security: controlled mounts, proper RO handling
- cleanup: standardize env vars CLAUDE_YOLO_* → CCYOLO_*
- Update VERSION to 0.4.0 in claude.sh
- Add v0.4.0 changelog entry with auth system refactor
…e workflow

- Use CLAUDE_YOLO_RELEASE_CMD if set
- Remove branch check from workflow
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Bug: `-c` Flag Change Breaks Existing Scripts

The -c flag's behavior is a breaking change. It was previously a shortcut for --claude authentication but now maps to --config DIR. This causes existing scripts using claude.sh -c (without an argument) to fail with "error: --config requires an argument". Using claude.sh -c somevalue will incorrectly interpret somevalue as a config directory. Additionally, when --config DIR is used, the script creates an empty {} in DIR/.claude.json if it doesn't exist, which may be an invalid or insufficient configuration for the Claude CLI.

claude.sh#L188-L223

https://github.com/thevibeworks/claude-code-yolo/blob/bf8334ae9bb79dce79c47e8c1c7e27e961fa1107/claude.sh#L188-L223

Fix in CursorFix in Web


BugBot free trial expires on July 22, 2025
You have used $0.00 of your $10.00 spend limit so far. Manage your spend limit in the Cursor dashboard.

Was this report helpful? Give feedback by reacting with 👍 or 👎

lroolle and others added 2 commits July 7, 2025 23:42
- Update VERSION in claude.sh to 0.4.1
- Add changelog entry with Docker compatibility improvements

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Add comprehensive changelog entry for v0.4.0 major refactor
- Document auth system overhaul, Docker architecture changes
- Include security improvements and breaking changes

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@lroolle
Copy link
Copy Markdown
Member Author

lroolle commented Jul 8, 2025

@claude list all issues and review

@claude-yolo
Copy link
Copy Markdown
Contributor

claude-yolo bot commented Jul 8, 2025

Claude finished @lroolle's task —— View job


Comprehensive PR Review

Todo List:

  • List all issues that this PR closes
  • Review all 11 changed files
  • Analyze the breaking change concern with -c flag
  • Check for bugs, security issues, performance problems
  • Provide comprehensive code review feedback

🚨 Critical Issues Found

1. BREAKING CHANGE: -c Flag Reassignment

File: claude.sh:205-223
Severity: HIGH - Will break existing scripts

The -c flag was changed from --claude (auth shortcut) to --config DIR (config directory). This breaks backward compatibility:

# Before: claude.sh -c  (worked - meant --claude auth)
# After:  claude.sh -c  (fails - requires argument for --config)

Solutions:

  • Use a different flag for config (e.g., --config-dir, --claude-config)
  • Keep -c for --claude auth and use long form only for config
  • Add deprecation warning if keeping breaking change

2. Issue Reference Problems

Issues: Claims to close #46, #31, #32, #24, #38

From codebase analysis:

🔍 Code Quality Issues

3. Security: Empty Config File Creation

File: claude.sh:214-217

if [ ! -f "$CONFIG_DIR/.claude.json" ]; then
    echo "Creating $CONFIG_DIR/.claude.json"
    echo '{}' >"$CONFIG_DIR/.claude.json"
fi

Issue: Creates invalid/empty Claude config that may cause authentication failures.
Fix: Check if this is sufficient or if proper Claude config structure is needed.

4. Path Expansion Edge Cases

File: claude.sh:209

CONFIG_DIR="${CONFIG_DIR/#\~/$HOME}"

Issue: Only handles ~ at start, not ~user or other shell expansions.
Fix: Use eval or realpath for robust path expansion.

5. Docker Volume Mount Logic

File: claude.sh:717-730
The custom config mounting logic is complex and potentially fragile:

for item in "$CONFIG_DIR"/.* "$CONFIG_DIR"/*; do
    if [ -e "$item" ]; then
        basename_item="$(basename "$item")"
        # Skip . and ..
        if [ "$basename_item" != "." ] && [ "$basename_item" != ".." ]; then

Issues:

  • Glob expansion with .* can be unreliable
  • No validation of what's being mounted
  • Could accidentally mount sensitive files

6. Environment Variable Handling

File: claude.sh:237-252

if [[ "$env_spec" == *"="* ]]; then
    EXTRA_ENV_VARS+=("-e" "$env_spec")
else
    env_value="${!env_spec}"
    if [ -n "$env_value" ]; then
        EXTRA_ENV_VARS+=("-e" "$env_spec=$env_value")
    else
        echo "warning: environment variable $env_spec not set, skipping" >&2
    fi
fi

Issues:

  • Indirect variable expansion ${!env_spec} can be dangerous
  • No validation of environment variable names
  • Could potentially expose sensitive variables

🏗️ Architecture Issues

7. Docker Path Consistency

Files: Dockerfile, docker-entrypoint.sh
The transition from /root to /home/claude paths is incomplete:

  • Some legacy /root references remain
  • Path handling is inconsistent between files
  • Could cause authentication mount failures

8. User Permission Management

File: docker-entrypoint.sh:77-94
Complex UID/GID management logic that's hard to debug:

if [ "$CLAUDE_GID" != "$current_gid" ]; then
    if getent group "$CLAUDE_GID" >/dev/null 2>&1; then
        existing_group=$(getent group "$CLAUDE_GID" | cut -d: -f1)
        usermod -g "$CLAUDE_GID" "$CLAUDE_USER" 2>/dev/null || true
    else
        groupmod -g "$CLAUDE_GID" "$CLAUDE_USER"
    fi
fi

✅ Positive Changes

Good Improvements:

  1. Custom config directory support: Addresses legitimate isolation needs
  2. Environment variable passthrough: Useful for containerized development
  3. Improved documentation: README updates are helpful
  4. Path standardization: Moving to /home/claude is cleaner
  5. Enhanced volume mounting: More flexible development setup

🔧 Recommendations

Immediate Fixes:

  1. Fix breaking change: Use different flag for config or provide migration path
  2. Update issue references: Remove non-existent issue numbers from PR description
  3. Validate config creation: Ensure created .claude.json is valid
  4. Improve error handling: Add validation for user inputs

Security Improvements:

  1. Validate mount sources: Check paths exist and are readable before mounting
  2. Sanitize environment variables: Validate env var names against allowlist
  3. Add permission checks: Verify user has access to config directories

Code Quality:

  1. Add unit tests: Test argument parsing and path handling logic
  2. Improve documentation: Add examples for new flags
  3. Refactor complex functions: Break down large functions in claude.sh
  4. Add error recovery: Handle Docker/mount failures gracefully

📊 Overall Assessment

Code Quality: ⚠️ Fair (needs improvement)
Security: ⚠️ Moderate concerns
Functionality: ✅ Good additions
Breaking Changes: 🚨 Critical issue

The PR adds valuable functionality but has a critical breaking change and several quality issues that should be addressed before merging.


Branch: refactor/envs → main
Job: View run

@claude-yolo
Copy link
Copy Markdown
Contributor

claude-yolo bot commented Jul 8, 2025

Claude finished @lroolle's task —— View job


Code Review Complete

Tasks:

Issues Analysis

Cannot access gh issue list due to tool restrictions, but PR claims to close #46, #31, #32, #24, #38. Based on the comprehensive changes for custom config and environment variable support, these likely relate to:

  • Custom config directory isolation
  • Environment variable passing
  • Auth isolation improvements
  • Docker path consistency

Code Review Findings

Critical Issue: Breaking Change in -c Flag (Confirmed)

Severity: HIGH - The cursor review is correct. This is a breaking change:

Before: claude.sh -c was shorthand for --claude (OAuth authentication)
After: claude.sh -c DIR requires a directory argument for --config

Impact:

  • claude.sh -c now fails with "error: --config requires an argument"
  • claude.sh -c somevalue interprets somevalue as config directory instead of treating -c as auth flag
  • Breaks existing scripts and user workflows

Files affected: claude.sh:205-223, claude-yolo:101-108

Recommendation: Use a different flag for config (e.g., --config-dir) or provide backward compatibility.

Architecture Review

Positive Changes:

  • ✅ Unified --auth-with pattern for auth method selection
  • ✅ Environment variable support with -e flag
  • ✅ Path standardization to /home/claude (claude.sh:709-744)
  • ✅ Proper model name translation for different auth modes
  • ✅ Security improvements with controlled auth file mounting

Code Quality:

  • ✅ Good error handling and validation
  • ✅ Comprehensive help documentation
  • ✅ Proper shell scripting practices with set -e
  • ✅ Safe directory checking with check_dangerous_directory()

Technical Issues Found

  1. Empty config creation (claude.sh:216): Creates {} in .claude.json if missing - may be insufficient for Claude CLI

  2. Volume mounting logic (claude.sh:717-729): Complex nested loop for config directory items could be simplified

  3. Environment variable handling (claude.sh:234-253): Could benefit from validation of environment variable names

Security Assessment

  • ✅ No malicious code detected
  • ✅ Proper user privilege handling with gosu
  • ✅ Dangerous directory warnings implemented
  • ✅ Read-only mounts where appropriate

Recommendations

  1. Fix Breaking Change: Either:

    • Use different flag for config (e.g., --config-home)
    • Add backward compatibility for -c as auth shortcut
    • Update documentation with migration guide
  2. Improve config initialization: Instead of empty {}, create proper Claude config structure

  3. Add validation: Validate environment variable names in -e flag processing

Conclusion

This is a substantial improvement to the project with good architectural changes, but the breaking change in the -c flag must be addressed before merging to avoid breaking existing user workflows.

--- • Branch: refactor/envs

@lroolle lroolle merged commit 7440457 into main Jul 9, 2025
4 checks passed
@lroolle lroolle deleted the refactor/envs branch July 9, 2025 17:00
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.

feat: enhanced custom config directory with --config/-c option

1 participant