Skip to content

[codex] Support CBORG Claude action setup#74

Draft
cmungall wants to merge 1 commit into
mainfrom
codex/support-cborg-artl-claude-actions
Draft

[codex] Support CBORG Claude action setup#74
cmungall wants to merge 1 commit into
mainfrom
codex/support-cborg-artl-claude-actions

Conversation

@cmungall
Copy link
Copy Markdown
Member

@cmungall cmungall commented May 2, 2026

Summary

  • add cborg_api_key plumbing to the reusable Claude Code composite action and issue triage wrapper
  • configure Claude Code to use the CBORG Anthropic-compatible endpoint
  • add optional ARTL MCP installation through uvx
  • add provenance comments and a timeout to the main Claude mention workflow

Validation

  • git diff --cached --check
  • parsed all GitHub workflow/action YAML files with uv run python in a clean worktree

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 2, 2026

Claude finished @cmungall's task —— View job


PR Review: Support CBORG Claude action setup

Reviewing the pull request for code quality, security, and best practices.

Todo List:

  • Read and analyze the modified action.yml files
  • Review the workflow changes in claude.yml
  • Check for security best practices in API key handling
  • Evaluate GitHub Actions conventions and syntax
  • Provide comprehensive review feedback

Critical Issues Found

🚨 Security Concerns

  1. API Key Masking Anti-pattern (claude-code-action/action.yml:158-163):

    env:
      ANTHROPIC_API_KEY: "."  # ⚠️ Security risk
      ANTHROPIC_AUTH_TOKEN: ${{ inputs.cborg_api_key }}
    • Setting ANTHROPIC_API_KEY: "." could mask authentication failures
    • Recommendation: Use conditional logic or proper placeholder
  2. Redundant API Key Exposure:

    • Action requires both anthropic_api_key and cborg_api_key but only uses cborg_api_key
    • Unnecessarily exposes additional secrets
    • Recommendation: Make anthropic_api_key optional when using CBORG

🔧 Code Quality Issues

  1. Hardcoded Configuration (claude-code-action/action.yml:162):

    ANTHROPIC_BASE_URL: "https://api.cborg.lbl.gov"  # Should be configurable
    • Recommendation: Add cborg_base_url input parameter for flexibility
  2. Duplicate Tool Arguments Logic (claude-code-action/action.yml:138,146):

    • The --allowedTools argument construction is duplicated and inconsistent
    • Uses both bash variable and inline template logic
    • Recommendation: Consolidate to use $ALLOWED_TOOLS_ARG consistently
  3. Missing Input Validation:

    • No validation that cborg_api_key is provided when using CBORG mode
    • Could cause runtime failures with cryptic error messages
  4. Dependency Assumptions:

    • install_artl_mcp uses uvx but doesn't verify uv availability
    • While uv is installed in step 47, this creates tight coupling

⚠️ Logic Issues

  1. Workflow Inconsistency (claude.yml):
    • The workflow still uses the old anthropics/claude-code-action@beta
    • Doesn't leverage the new CBORG-enabled local action
    • Recommendation: Update to use local action for CBORG support

✅ Positive Aspects

  • Good Documentation: Clear comments explaining CBORG vs Claude Code token differences
  • Proper Timeouts: Added timeout to main workflow (line 26)
  • Clean YAML Structure: Well-formatted and readable action definitions
  • Backward Compatibility: Maintains existing functionality while adding new features

Recommendations

  1. Make API keys conditional:

    anthropic_api_key:
      description: "Anthropic API key (required when not using CBORG)"
      required: false
  2. Add input validation step:

    if [[ -z "${{ inputs.anthropic_api_key }}" && -z "${{ inputs.cborg_api_key }}" ]]; then
      echo "::error::Either anthropic_api_key or cborg_api_key must be provided"
      exit 1
    fi
  3. Make base URL configurable:

    cborg_base_url:
      description: "CBORG API base URL"
      required: false
      default: "https://api.cborg.lbl.gov"
  4. Fix security anti-pattern:

    env:
      ANTHROPIC_API_KEY: ${{ inputs.cborg_api_key && '.' || inputs.anthropic_api_key }}
      ANTHROPIC_AUTH_TOKEN: ${{ inputs.cborg_api_key }}

Overall Assessment

This PR adds valuable CBORG integration but needs security and robustness improvements before merging. The core functionality appears sound, but the implementation has several issues that could lead to security vulnerabilities and runtime failures.

Recommended Action: Request changes to address security concerns and improve input validation before approval.

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.

1 participant