feat: add Cursor CLI installation attempts documentation and enhance … #366
Conversation
…Docker setup - Introduced a new markdown file summarizing various attempts to install the Cursor CLI in Docker, detailing approaches, results, and key learnings. - Updated Dockerfile to ensure proper installation of Cursor CLI for the non-root user, including necessary PATH adjustments for interactive shells. - Enhanced entrypoint script to manage OAuth tokens for both Claude and Cursor CLIs, ensuring correct permissions and directory setups. - Added scripts for extracting OAuth tokens from macOS Keychain and Linux JSON files for seamless integration with Docker. - Updated docker-compose files to support persistent storage for CLI configurations and authentication tokens. These changes improve the development workflow and provide clear guidance on CLI installation and authentication processes.
|
Warning Rate limit exceeded@webdevcody has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 8 minutes and 47 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughMigrates container base to Debian-slim, installs Claude/Cursor CLIs, creates a non-root automaker user with persisted CLI config and env-driven credential injection, adds host token extraction scripts, updates entrypoint to write credentials, and adds logic to detect and conditionally rebuild Docker images. Changes
Sequence DiagramsequenceDiagram
autonumber
actor Developer
participant Host as Host OS
participant Scripts as Token Scripts
participant Docker as Docker Engine
participant Image as Built Image
participant Container as Server Container
participant Entrypoint as docker-entrypoint.sh
participant CLIs as Claude & Cursor CLIs
participant UI as Web UI
Developer->>Scripts: run get-claude-token / get-cursor-token
Scripts->>Host: query Keychain or config files
Host-->>Scripts: return credentials/token
Scripts->>Developer: output JSON/token
Developer->>Docker: docker-compose up (env or mounts)
Docker->>Image: start / build (conditional)
Image->>Container: create & start
Container->>Entrypoint: execute entrypoint
Entrypoint->>Entrypoint: ensure dirs & perms for automaker
alt CLAUDE_OAUTH_CREDENTIALS present
Entrypoint->>Container: write .claude/.credentials.json (600) owned by automaker
end
alt CURSOR_AUTH_TOKEN present
Entrypoint->>Container: write .config/cursor/auth.json (600) owned by automaker
end
Entrypoint->>CLIs: initialize CLIs with persisted creds
CLIs-->>Container: CLIs ready
UI->>Container: api.setup.getClaudeStatus
Container-->>UI: return mapped Claude auth status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @webdevcody, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refines the Docker development experience by enhancing the integration and authentication of both Cursor and Claude CLIs. It introduces a more robust Docker setup, including a base image change and improved dependency management. Key changes involve streamlining how OAuth tokens are handled, allowing them to be extracted from host systems (macOS Keychain, Linux JSON) and securely passed into the Docker container, ensuring persistent authentication. Furthermore, the PR adds intelligent logic to automatically rebuild Docker images when source files change and updates the UI to reflect Claude CLI authentication status, all contributing to a smoother and more efficient development workflow within the Dockerized environment. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly enhances the Docker setup by adding support for Cursor CLI installation and authentication. It introduces scripts for extracting OAuth tokens, updates the Dockerfile for proper installation, and enhances the entrypoint script to manage tokens. The changes improve the development workflow and provide clearer documentation. My review includes suggestions to reduce Docker image layers, refactor duplicated code in a React hook and build scripts, simplify shell script logic, and improve documentation clarity and consistency.
| # Add to automaker's .bashrc for bash interactive shells | ||
| RUN echo 'export PATH="/home/automaker/.local/bin:$PATH"' >> /home/automaker/.bashrc && \ | ||
| chown automaker:automaker /home/automaker/.bashrc | ||
|
|
||
| # Also add to root's .bashrc since docker exec defaults to root | ||
| RUN echo 'export PATH="/home/automaker/.local/bin:$PATH"' >> /root/.bashrc |
There was a problem hiding this comment.
To reduce Docker image layers and group related commands, you can combine the RUN instructions for updating .bashrc files into a single instruction. This makes the Dockerfile more efficient and easier to maintain.
# Add to automaker's and root's .bashrc for bash interactive shells
RUN echo 'export PATH="/home/automaker/.local/bin:$PATH"' >> /home/automaker/.bashrc && \
chown automaker:automaker /home/automaker/.bashrc && \
echo 'export PATH="/home/automaker/.local/bin:$PATH"' >> /root/.bashrc
| // Also refresh auth status | ||
| if (api?.setup?.getClaudeStatus) { | ||
| try { | ||
| const result = await api.setup.getClaudeStatus(); | ||
| if (result.success && result.auth) { | ||
| const auth = result.auth as typeof result.auth & { | ||
| oauthTokenValid?: boolean; | ||
| apiKeyValid?: boolean; | ||
| }; | ||
| const validMethods = [ | ||
| 'oauth_token_env', | ||
| 'oauth_token', | ||
| 'api_key', | ||
| 'api_key_env', | ||
| 'credentials_file', | ||
| 'cli_authenticated', | ||
| 'none', | ||
| ] as const; | ||
| type AuthMethod = (typeof validMethods)[number]; | ||
| const method: AuthMethod = validMethods.includes(auth.method as AuthMethod) | ||
| ? (auth.method as AuthMethod) | ||
| : auth.authenticated | ||
| ? 'api_key' | ||
| : 'none'; | ||
| const authStatus = { | ||
| authenticated: auth.authenticated, | ||
| method, | ||
| hasCredentialsFile: auth.hasCredentialsFile ?? false, | ||
| oauthTokenValid: | ||
| auth.oauthTokenValid || auth.hasStoredOAuthToken || auth.hasEnvOAuthToken, | ||
| apiKeyValid: auth.apiKeyValid || auth.hasStoredApiKey || auth.hasEnvApiKey, | ||
| hasEnvOAuthToken: auth.hasEnvOAuthToken, | ||
| hasEnvApiKey: auth.hasEnvApiKey, | ||
| }; | ||
| setClaudeAuthStatus(authStatus); | ||
| } | ||
| } catch (error) { | ||
| logger.error('Failed to refresh Claude auth status:', error); | ||
| } | ||
| } |
There was a problem hiding this comment.
The logic to refresh the Claude auth status inside handleRefreshClaudeCli is a duplicate of the logic inside the useEffect hook (lines 51-92). To improve maintainability and avoid code duplication, this logic should be extracted into a separate useCallback function. This new function can then be called from both useEffect and handleRefreshClaudeCli.
dev.mjs
Outdated
| let serverImageName, uiImageName; | ||
| try { | ||
| const composeConfig = execSync('docker compose config --format json', { | ||
| encoding: 'utf-8', | ||
| cwd: __dirname, | ||
| }); | ||
| const config = JSON.parse(composeConfig); | ||
|
|
||
| // Docker Compose generates image names as <project>_<service> | ||
| // Get project name from config or default to directory name | ||
| const projectName = | ||
| config.name || | ||
| path | ||
| .basename(__dirname) | ||
| .toLowerCase() | ||
| .replace(/[^a-z0-9]/g, ''); | ||
| serverImageName = `${projectName}_server`; | ||
| uiImageName = `${projectName}_ui`; | ||
| } catch (error) { | ||
| // Fallback to default naming convention | ||
| const projectName = path | ||
| .basename(__dirname) | ||
| .toLowerCase() | ||
| .replace(/[^a-z0-9]/g, ''); | ||
| serverImageName = `${projectName}_server`; | ||
| uiImageName = `${projectName}_ui`; | ||
| } |
There was a problem hiding this comment.
There's code duplication in how projectName, serverImageName, and uiImageName are determined. The logic for generating these names is repeated in both the try and catch blocks. You can refactor this to avoid repetition and improve readability.
let serverImageName, uiImageName;
let projectName;
try {
const composeConfig = execSync('docker compose config --format json', {
encoding: 'utf-8',
cwd: __dirname,
});
const config = JSON.parse(composeConfig);
projectName = config.name;
} catch (error) {
// Fallback to default naming convention below
}
// Use project name from config or default to directory name
const finalProjectName =
projectName ||
path
.basename(__dirname)
.toLowerCase()
.replace(/[^a-z0-9]/g, '');
serverImageName = `${finalProjectName}_server`;
uiImageName = `${finalProjectName}_ui`;
docker-entrypoint.sh
Outdated
| # Fix permissions on Cursor CLI config directory if it exists | ||
| # This handles the case where a volume is mounted and owned by root | ||
| if [ -d "/home/automaker/.claude" ]; then | ||
| chown -R automaker:automaker /home/automaker/.claude | ||
| chmod -R 755 /home/automaker/.claude | ||
| if [ -d "/home/automaker/.cursor" ]; then | ||
| chown -R automaker:automaker /home/automaker/.cursor | ||
| chmod -R 700 /home/automaker/.cursor | ||
| fi | ||
|
|
||
| # Ensure the directory exists with correct permissions if volume is empty | ||
| if [ ! -d "/home/automaker/.claude" ]; then | ||
| mkdir -p /home/automaker/.claude | ||
| chown automaker:automaker /home/automaker/.claude | ||
| chmod 755 /home/automaker/.claude | ||
| if [ ! -d "/home/automaker/.cursor" ]; then | ||
| mkdir -p /home/automaker/.cursor | ||
| chown automaker:automaker /home/automaker/.cursor | ||
| chmod 700 /home/automaker/.cursor | ||
| fi |
There was a problem hiding this comment.
This block of code for setting up the .cursor directory is a bit redundant. It checks for the directory's existence, sets permissions, and then checks for its non-existence to create it and set permissions again. This can be simplified into a single, more logical block that handles both cases (mounted volume or new directory).
| # Fix permissions on Cursor CLI config directory if it exists | |
| # This handles the case where a volume is mounted and owned by root | |
| if [ -d "/home/automaker/.claude" ]; then | |
| chown -R automaker:automaker /home/automaker/.claude | |
| chmod -R 755 /home/automaker/.claude | |
| if [ -d "/home/automaker/.cursor" ]; then | |
| chown -R automaker:automaker /home/automaker/.cursor | |
| chmod -R 700 /home/automaker/.cursor | |
| fi | |
| # Ensure the directory exists with correct permissions if volume is empty | |
| if [ ! -d "/home/automaker/.claude" ]; then | |
| mkdir -p /home/automaker/.claude | |
| chown automaker:automaker /home/automaker/.claude | |
| chmod 755 /home/automaker/.claude | |
| if [ ! -d "/home/automaker/.cursor" ]; then | |
| mkdir -p /home/automaker/.cursor | |
| chown automaker:automaker /home/automaker/.cursor | |
| chmod 700 /home/automaker/.cursor | |
| fi | |
| # Ensure Cursor CLI config directory exists with correct permissions | |
| if [ ! -d "/home/automaker/.cursor" ]; then | |
| mkdir -p /home/automaker/.cursor | |
| fi | |
| chown -R automaker:automaker /home/automaker/.cursor | |
| chmod -R 700 /home/automaker/.cursor |
| **Option 2: Bind mount credential directories directly** | ||
|
|
||
| ```yaml | ||
| # In docker-compose.override.yml | ||
| volumes: | ||
| - ~/.claude:/home/automaker/.claude | ||
| - ~/.config/cursor:/home/automaker/.config/cursor | ||
| ``` |
There was a problem hiding this comment.
There appears to be an inconsistency in the documentation for mounting Cursor CLI directories. Here, you suggest mounting ~/.config/cursor, but in docker-compose.override.yml.example and other parts of this document, ~/.cursor is mentioned. Please clarify which directory should be mounted or if both are needed for different purposes (e.g., auth vs. config) to avoid confusion.
start.mjs
Outdated
| let serverImageName, uiImageName; | ||
| try { | ||
| const composeConfig = execSync('docker compose config --format json', { | ||
| encoding: 'utf-8', | ||
| cwd: __dirname, | ||
| }); | ||
| const config = JSON.parse(composeConfig); | ||
|
|
||
| // Docker Compose generates image names as <project>_<service> | ||
| // Get project name from config or default to directory name | ||
| const projectName = | ||
| config.name || | ||
| path | ||
| .basename(__dirname) | ||
| .toLowerCase() | ||
| .replace(/[^a-z0-9]/g, ''); | ||
| serverImageName = `${projectName}_server`; | ||
| uiImageName = `${projectName}_ui`; | ||
| } catch (error) { | ||
| // Fallback to default naming convention | ||
| const projectName = path | ||
| .basename(__dirname) | ||
| .toLowerCase() | ||
| .replace(/[^a-z0-9]/g, ''); | ||
| serverImageName = `${projectName}_server`; | ||
| uiImageName = `${projectName}_ui`; | ||
| } |
There was a problem hiding this comment.
There's code duplication in how projectName, serverImageName, and uiImageName are determined. The logic for generating these names is repeated in both the try and catch blocks. You can refactor this to avoid repetition and improve readability.
let serverImageName, uiImageName;
let projectName;
try {
const composeConfig = execSync('docker compose config --format json', {
encoding: 'utf-8',
cwd: __dirname,
});
const config = JSON.parse(composeConfig);
projectName = config.name;
} catch (error) {
// Fallback to default naming convention below
}
// Use project name from config or default to directory name
const finalProjectName =
projectName ||
path
.basename(__dirname)
.toLowerCase()
.replace(/[^a-z0-9]/g, '');
serverImageName = `${finalProjectName}_server`;
uiImageName = `${finalProjectName}_ui`;![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Fix all issues with AI Agents 🤖
In @docker-compose.override.yml.example:
- Around line 25-29: The env var name is inconsistent: the example uses
CURSOR_API_KEY while docker-compose.yml and scripts/get-cursor-token.sh use
CURSOR_AUTH_TOKEN; update the example line to use CURSOR_AUTH_TOKEN (or vice
versa everywhere) so the extraction script, docker-compose.yml and
docker-compose.override.yml.example all reference the same variable name
(CURSOR_AUTH_TOKEN) and ensure any documentation/comments mention that exact
symbol.
In @docker-entrypoint.sh:
- Around line 37-48: The script writes CURSOR_AUTH_TOKEN directly into auth.json
which can produce invalid JSON if the token contains quotes, backslashes or
newlines; update the docker-entrypoint.sh logic that sets CURSOR_CONFIG_DIR and
writes auth.json to first escape the token (e.g., store an ESCAPED_TOKEN derived
from CURSOR_AUTH_TOKEN by escaping " and \ and/or stripping newlines) and then
write the JSON using the escaped value for "accessToken", or alternatively
validate that CURSOR_AUTH_TOKEN matches a safe charset (alphanumeric, -, _) and
fail early; ensure the chmod/chown steps remain unchanged and reference
CURSOR_AUTH_TOKEN, CURSOR_CONFIG_DIR and the auth.json write block.
In @Dockerfile:
- Around line 85-94: Avoid the curl|bash pipeline: instead fetch the installer
artifact first, verify its authenticity (checksum or signature) and only then
execute it as the automaker user; update the Dockerfile section that currently
uses "USER automaker"/"ENV HOME=/home/automaker" and the RUN that pipes curl to
bash to download the installer to a temporary file, validate a pinned version or
checksum/signature, then run that local script as automaker, and finally check
installation with "which cursor-agent" and "cursor-agent --version"; if no
signed artifact or pinned URL exists, document the risk and pin the installer
version or source before executing.
In @start.mjs:
- Around line 106-115: config.name is used directly to build serverImageName and
uiImageName and passed into execSync, enabling command injection; sanitize
config.name the same way as the fallback path before constructing
serverImageName/uiImageName and before any shell interpolation (i.e., convert to
lowercase and strip characters not matching [a-z0-9] using the existing regex),
then use that sanitized project name when forming the image names and in the
execSync calls that inspect the images.
🧹 Nitpick comments (4)
apps/ui/src/components/views/settings-view/hooks/use-cli-status.ts (1)
107-146: Extract duplicate auth status mapping logic into a helper function.The auth mapping logic here (lines 107-146) duplicates the logic in the
useEffect(lines 51-92). Consider extracting into a shared helper to improve maintainability:🔎 Proposed refactor
// Add helper function outside the hook or at the top of useCliStatus function mapAuthStatusFromResult(auth: { authenticated: boolean; method: string; hasCredentialsFile?: boolean; oauthTokenValid?: boolean; apiKeyValid?: boolean; hasStoredOAuthToken?: boolean; hasEnvOAuthToken?: boolean; hasStoredApiKey?: boolean; hasEnvApiKey?: boolean; }) { const validMethods = [ 'oauth_token_env', 'oauth_token', 'api_key', 'api_key_env', 'credentials_file', 'cli_authenticated', 'none', ] as const; type AuthMethod = (typeof validMethods)[number]; const method: AuthMethod = validMethods.includes(auth.method as AuthMethod) ? (auth.method as AuthMethod) : auth.authenticated ? 'api_key' : 'none'; return { authenticated: auth.authenticated, method, hasCredentialsFile: auth.hasCredentialsFile ?? false, oauthTokenValid: auth.oauthTokenValid || auth.hasStoredOAuthToken || auth.hasEnvOAuthToken, apiKeyValid: auth.apiKeyValid || auth.hasStoredApiKey || auth.hasEnvApiKey, hasEnvOAuthToken: auth.hasEnvOAuthToken, hasEnvApiKey: auth.hasEnvApiKey, }; } // Then use in both places: // setClaudeAuthStatus(mapAuthStatusFromResult(auth));docker-entrypoint.sh (1)
22-25:chmod -R 700sets execute bit on files unnecessarily.Using
chmod -R 700applies 700 to both directories and files. Credential files should be 600 (no execute), while directories should be 700. Consider separating directory and file permissions.Proposed fix
if [ -d "/home/automaker/.cursor" ]; then chown -R automaker:automaker /home/automaker/.cursor - chmod -R 700 /home/automaker/.cursor + chmod 700 /home/automaker/.cursor + find /home/automaker/.cursor -type f -exec chmod 600 {} \; fistart.mjs (1)
62-140: ExtractshouldRebuildDockerImagesto shared module to eliminate duplication.This function is duplicated verbatim in
dev.mjs. Extract it toscripts/launcher-utils.mjsto maintain a single source of truth and reduce maintenance burden.Dockerfile (1)
74-75: Pin Claude CLI version for reproducible builds.The
@anthropic-ai/claude-codepackage is installed without a version constraint. This can lead to non-reproducible builds and unexpected behavior if a breaking change is released. The latest available version is 1.0.100.Proposed fix
# Install Claude CLI globally (available to all users via npm global bin) -RUN npm install -g @anthropic-ai/claude-code +RUN npm install -g @anthropic-ai/claude-code@1.0.100
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
Dockerfileapps/ui/src/components/views/settings-view/hooks/use-cli-status.tsdev.mjsdocker-compose.override.yml.exampledocker-compose.ymldocker-entrypoint.shdocs/docker-isolation.mdlibs/platform/src/system-paths.tsscripts/get-claude-token.shscripts/get-cursor-token.shstart.mjs
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always import from shared packages (@automaker/*), never from old relative paths
Files:
libs/platform/src/system-paths.tsapps/ui/src/components/views/settings-view/hooks/use-cli-status.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
resolveModelString()from @automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names
Files:
libs/platform/src/system-paths.tsapps/ui/src/components/views/settings-view/hooks/use-cli-status.ts
🧬 Code graph analysis (1)
start.mjs (3)
dev.mjs (20)
dockerfilePath(55-55)__dirname(37-37)packageJsonPath(56-56)dockerfileMtime(59-59)packageJsonMtime(60-60)latestSourceMtime(61-61)serverImageName(64-64)composeConfig(66-69)config(70-70)projectName(74-79)projectName(84-87)needsRebuild(93-93)needsRebuild(262-262)serverImageInfo(97-100)uiImageInfo(103-106)serverCreated(114-114)uiCreated(115-115)oldestImageTime(116-116)buildFlag(263-263)crossSpawn(34-34)libs/platform/src/secure-fs.ts (1)
statSync(392-395)scripts/launcher-utils.mjs (2)
log(88-90)crossSpawn(24-24)
🪛 markdownlint-cli2 (0.18.1)
docs/docker-isolation.md
96-96: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
103-103: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (11)
libs/platform/src/system-paths.ts (1)
803-812: LGTM! Well-documented credential format expansion.The multi-format credential detection is cleanly implemented with clear comments explaining the three supported formats. The optional chaining on
credentials.claudeAiOauth?.accessTokensafely handles missing nested properties.scripts/get-claude-token.sh (1)
1-34: LGTM! Clean macOS Keychain extraction script.The script correctly validates platform and prerequisites before attempting credential extraction. Error messages provide helpful guidance for users.
scripts/get-cursor-token.sh (1)
1-69: LGTM! Well-structured cross-platform token extraction.The script properly handles platform-specific credential storage (Keychain on macOS, JSON file on Linux) with appropriate prerequisite checks and helpful error messages. The XDG_CONFIG_HOME fallback on Linux follows the XDG Base Directory Specification correctly.
docs/docker-isolation.md (1)
60-111: LGTM! Comprehensive CLI authentication documentation.The documentation clearly explains platform-specific token extraction methods and provides actionable commands. The two options for Linux/Windows (env vars vs bind mounts) give users flexibility based on their preferences.
docker-compose.yml (2)
39-48: LGTM! Well-documented environment variables for CLI credentials.The new environment variables follow the established pattern with sensible empty defaults. The comments provide clear guidance on how to obtain the tokens using the companion scripts.
77-79: LGTM! Cursor configuration volume setup.The volume mount at
/home/automaker/.cursoraligns with the non-root user home directory established in the Dockerfile, and follows the same pattern used for Claude CLI configuration.Also applies to: 100-103
docker-entrypoint.sh (1)
1-51: LGTM overall - credential handling follows good security practices.The entrypoint properly creates directories with restrictive permissions (700), sets credential files to 600, and uses
gosufor user switching. The ownership enforcement ensures mounted volumes work correctly with the non-root user.start.mjs (1)
319-328: LGTM - Rebuild detection integration is clean.The conditional logging and
buildFlagconstruction provide clear user feedback about what's happening. The fallback behavior (rebuild when uncertain) is appropriately conservative.dev.mjs (1)
260-270: LGTM - Consistent with start.mjs implementation.The rebuild detection and user messaging follow the same pattern as
start.mjs, maintaining consistency across dev and production modes.Dockerfile (2)
56-72: LGTM - GitHub CLI installation with multi-arch support.Good practice pinning
GH_VERSION="2.63.2"and handling both amd64 and arm64 architectures with proper cleanup.
77-83: LGTM - User setup and PATH configuration are comprehensive.The automaker user is created correctly with appropriate home directory structure. PATH is configured in multiple locations (profile.d, .bashrc for both users, ENV) ensuring cursor-agent is accessible in all shell contexts.
Also applies to: 96-106, 143-144
| # ===== macOS Users ===== | ||
| # On macOS, OAuth tokens are stored in SQLite databases, not plain files. | ||
| # Extract your Cursor token with: ./scripts/get-cursor-token.sh | ||
| # Then set it here or in your .env file: | ||
| # - CURSOR_API_KEY=${CURSOR_API_KEY:-} |
There was a problem hiding this comment.
Environment variable name inconsistency.
Line 29 references CURSOR_API_KEY, but docker-compose.yml defines CURSOR_AUTH_TOKEN and the extraction script (scripts/get-cursor-token.sh) also uses CURSOR_AUTH_TOKEN.
🔎 Proposed fix
# Extract your Cursor token with: ./scripts/get-cursor-token.sh
# Then set it here or in your .env file:
- # - CURSOR_API_KEY=${CURSOR_API_KEY:-}
+ # - CURSOR_AUTH_TOKEN=${CURSOR_AUTH_TOKEN:-}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # ===== macOS Users ===== | |
| # On macOS, OAuth tokens are stored in SQLite databases, not plain files. | |
| # Extract your Cursor token with: ./scripts/get-cursor-token.sh | |
| # Then set it here or in your .env file: | |
| # - CURSOR_API_KEY=${CURSOR_API_KEY:-} | |
| # ===== macOS Users ===== | |
| # On macOS, OAuth tokens are stored in SQLite databases, not plain files. | |
| # Extract your Cursor token with: ./scripts/get-cursor-token.sh | |
| # Then set it here or in your .env file: | |
| # - CURSOR_AUTH_TOKEN=${CURSOR_AUTH_TOKEN:-} |
🤖 Prompt for AI Agents
In @docker-compose.override.yml.example around lines 25-29, The env var name is
inconsistent: the example uses CURSOR_API_KEY while docker-compose.yml and
scripts/get-cursor-token.sh use CURSOR_AUTH_TOKEN; update the example line to
use CURSOR_AUTH_TOKEN (or vice versa everywhere) so the extraction script,
docker-compose.yml and docker-compose.override.yml.example all reference the
same variable name (CURSOR_AUTH_TOKEN) and ensure any documentation/comments
mention that exact symbol.
| # Install Cursor CLI as the automaker user | ||
| # Set HOME explicitly and install to /home/automaker/.local/bin/ | ||
| USER automaker | ||
| ENV HOME=/home/automaker | ||
| RUN curl https://cursor.com/install -fsS | bash && \ | ||
| echo "=== Checking Cursor CLI installation ===" && \ | ||
| ls -la /home/automaker/.local/bin/ && \ | ||
| echo "=== PATH is: $PATH ===" && \ | ||
| (which cursor-agent && cursor-agent --version) || echo "cursor-agent installed (may need auth setup)" | ||
| USER root |
There was a problem hiding this comment.
Curl-pipe-to-bash pattern introduces supply chain risk.
Piping remote scripts directly to bash (curl ... | bash) is a security anti-pattern as it executes arbitrary code without verification. If cursor.com is compromised or serves different content, the build could be affected.
Consider:
- Downloading the script first and inspecting/checksumming it
- Pinning to a specific installer version if available
- Documenting this as an accepted risk if no alternative exists
🤖 Prompt for AI Agents
In @Dockerfile around lines 85-94, Avoid the curl|bash pipeline: instead fetch
the installer artifact first, verify its authenticity (checksum or signature)
and only then execute it as the automaker user; update the Dockerfile section
that currently uses "USER automaker"/"ENV HOME=/home/automaker" and the RUN that
pipes curl to bash to download the installer to a temporary file, validate a
pinned version or checksum/signature, then run that local script as automaker,
and finally check installation with "which cursor-agent" and "cursor-agent
--version"; if no signed artifact or pinned URL exists, document the risk and
pin the installer version or source before executing.
- Moved the logger creation outside the hook to prevent infinite re-renders. - Updated dependencies in the checkStatus function to remove logger from the dependency array. These changes enhance performance and maintainability of the useCliStatus hook.
- Added a `sanitizeProjectName` function to ensure project names are safe for shell commands and Docker image names by converting them to lowercase and removing non-alphanumeric characters. - Updated `dev.mjs` and `start.mjs` to utilize the new sanitization function when determining Docker image names, enhancing security and consistency. - Refactored the Docker entrypoint script to ensure proper permissions for the Cursor CLI config directory, improving setup reliability. - Clarified documentation regarding the storage location of OAuth tokens for the Cursor CLI on Linux. These changes improve the robustness of the Docker setup and enhance the overall development workflow.
…ker into cursor-docker-oauth
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @apps/ui/src/components/views/setup-view/hooks/use-cli-status.ts:
- Around line 10-14: Remove the duplicate declaration of the logger constant:
keep a single const logger = createLogger('CliStatus') declaration (the one
intended "outside the hook") and delete the other redeclaration so there is only
one logger variable in use-cli-status (referencing the createLogger('CliStatus')
call and the logger identifier).
In @docker-entrypoint.sh:
- Around line 31-42: The current docker-entrypoint.sh writes CURSOR_AUTH_TOKEN
directly into the heredoc for "$CURSOR_CONFIG_DIR/auth.json", which can produce
invalid JSON if the token contains quotes, backslashes, or newlines; instead
serialize/escape the token when producing the JSON (e.g., replace the heredoc
write with a JSON-safe serialization using jq or Python's json.dumps) so that
CURSOR_AUTH_TOKEN is properly escaped before writing to auth.json, and keep the
mkdir, chmod 600 and chown automaker:automaker steps for CURSOR_CONFIG_DIR.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
start.mjs (1)
1-1: Address Prettier formatting issues.The pipeline indicates Prettier formatting issues in this file. Run
prettier --write start.mjsto fix.dev.mjs (1)
1-1: Address Prettier formatting issues.The pipeline indicates Prettier formatting issues in this file. Run
prettier --write dev.mjsto fix.
🧹 Nitpick comments (4)
apps/ui/src/components/views/settings-view/hooks/use-cli-status.ts (1)
5-6: Import statement should be grouped with other imports at the top.The import on line 6 appears after the
const loggerdeclaration on line 5. Move the import to the top of the file with the other imports for consistent organization.🔎 Proposed fix
import { useState, useEffect, useCallback } from 'react'; import { createLogger } from '@automaker/utils/logger'; import { useSetupStore } from '@/store/setup-store'; +import { getElectronAPI } from '@/lib/electron'; const logger = createLogger('CliStatus'); -import { getElectronAPI } from '@/lib/electron';docs/docker-isolation.md (1)
96-103: Consider using proper headings instead of bold emphasis.The static analysis tool flags lines 96 and 103 as using emphasis instead of headings. For better document structure and accessibility, consider converting these to proper markdown headings.
🔎 Proposed fix
-**Option 1: Extract tokens to environment variables (recommended)** +### Option 1: Extract tokens to environment variables (recommended) ```bash # Linux: Extract tokens to .env echo "CURSOR_AUTH_TOKEN=$(jq -r '.accessToken' ~/.config/cursor/auth.json)" >> .env-Option 2: Bind mount credential directories directly
+### Option 2: Bind mount credential directories directly</details> </blockquote></details> <details> <summary>start.mjs (1)</summary><blockquote> `59-140`: **Consider extracting shared utilities to avoid code duplication.** The `sanitizeProjectName` and `shouldRebuildDockerImages` functions are duplicated verbatim in both `start.mjs` and `dev.mjs`. Consider extracting these to `scripts/launcher-utils.mjs` to maintain a single source of truth. </blockquote></details> <details> <summary>dev.mjs (1)</summary><blockquote> `50-131`: **Code duplication with start.mjs.** As noted in the start.mjs review, `sanitizeProjectName` and `shouldRebuildDockerImages` are identical in both files. Extract to a shared module in `scripts/launcher-utils.mjs`. </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 2a0719e00c07cc1ae34ae353d977ca6de7dc4302 and 49f32c4d59b48c7b83a8a4f7ed678953a3f47fd1. </details> <details> <summary>📒 Files selected for processing (6)</summary> * `apps/ui/src/components/views/settings-view/hooks/use-cli-status.ts` * `apps/ui/src/components/views/setup-view/hooks/use-cli-status.ts` * `dev.mjs` * `docker-entrypoint.sh` * `docs/docker-isolation.md` * `start.mjs` </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>📓 Path-based instructions (2)</summary> <details> <summary>**/*.{ts,tsx,js,jsx}</summary> **📄 CodeRabbit inference engine (CLAUDE.md)** > Always import from shared packages (@automaker/*), never from old relative paths Files: - `apps/ui/src/components/views/settings-view/hooks/use-cli-status.ts` - `apps/ui/src/components/views/setup-view/hooks/use-cli-status.ts` </details> <details> <summary>**/*.{ts,tsx}</summary> **📄 CodeRabbit inference engine (CLAUDE.md)** > Use `resolveModelString()` from @automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names Files: - `apps/ui/src/components/views/settings-view/hooks/use-cli-status.ts` - `apps/ui/src/components/views/setup-view/hooks/use-cli-status.ts` </details> </details><details> <summary>🧬 Code graph analysis (2)</summary> <details> <summary>apps/ui/src/components/views/settings-view/hooks/use-cli-status.ts (1)</summary><blockquote> <details> <summary>apps/ui/src/lib/electron.ts (1)</summary> * `getElectronAPI` (839-848) </details> </blockquote></details> <details> <summary>apps/ui/src/components/views/setup-view/hooks/use-cli-status.ts (2)</summary><blockquote> <details> <summary>libs/utils/src/logger.ts (1)</summary> * `createLogger` (127-219) </details> <details> <summary>libs/utils/src/index.ts (1)</summary> * `createLogger` (44-44) </details> </blockquote></details> </details><details> <summary>🪛 Biome (2.1.2)</summary> <details> <summary>apps/ui/src/components/views/setup-view/hooks/use-cli-status.ts</summary> [error] 13-13: Shouldn't redeclare 'logger'. Consider to delete it or rename it. 'logger' is defined here: (lint/suspicious/noRedeclare) </details> </details> <details> <summary>🪛 GitHub Actions: Format Check</summary> <details> <summary>start.mjs</summary> [warning] 1-1: Prettier formatting issues found in this file. Run 'prettier --write' to fix. </details> <details> <summary>dev.mjs</summary> [warning] 1-1: Prettier formatting issues found in this file. Run 'prettier --write' to fix. </details> </details> <details> <summary>🪛 GitHub Actions: PR Build Check</summary> <details> <summary>apps/ui/src/components/views/setup-view/hooks/use-cli-status.ts</summary> [error] 13-13: The symbol "logger" has already been declared. </details> </details> <details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>docs/docker-isolation.md</summary> 96-96: Emphasis used instead of a heading (MD036, no-emphasis-as-heading) --- 103-103: Emphasis used instead of a heading (MD036, no-emphasis-as-heading) </details> </details> </details> <details> <summary>🔇 Additional comments (7)</summary><blockquote> <details> <summary>apps/ui/src/components/views/setup-view/hooks/use-cli-status.ts (1)</summary><blockquote> `23-72`: **Callback logic looks correct.** The `checkStatus` implementation properly validates auth methods, constructs status objects, and handles errors. The dependency array correctly excludes `logger` since it's now module-scoped (once the duplicate declaration is fixed). </blockquote></details> <details> <summary>apps/ui/src/components/views/settings-view/hooks/use-cli-status.ts (2)</summary><blockquote> `35-80`: **Good extraction of shared auth refresh logic.** The `refreshAuthStatus` callback properly centralizes the auth status fetching logic, addressing the code duplication concern. The method validation, field mapping, and error handling are well-implemented. --- `83-120`: **LGTM on the shared refresh logic usage.** Both `useEffect` and `handleRefreshClaudeCli` now properly use the shared `refreshAuthStatus` callback, eliminating the duplication flagged in previous reviews. The dependency arrays are correct. </blockquote></details> <details> <summary>docker-entrypoint.sh (1)</summary><blockquote> `4-26`: **LGTM on the directory setup logic.** The consolidated approach for creating directories, setting permissions, and fixing ownership is clean and handles both mounted volumes and fresh directories correctly. The use of `chmod 700` for directories and `chmod 600` for credential files follows security best practices. </blockquote></details> <details> <summary>start.mjs (2)</summary><blockquote> `106-115`: **LGTM on command injection mitigation.** The previous concern about command injection from `config.name` has been addressed. The `sanitizeProjectName` function now ensures that project names only contain lowercase alphanumeric characters before being used in shell commands, preventing injection attacks. --- `319-347`: **LGTM on conditional Docker rebuild logic.** The integration of `shouldRebuildDockerImages` with appropriate logging and conditional `--build` flag is well implemented. The fallback to rebuilding on errors is a safe default. </blockquote></details> <details> <summary>dev.mjs (1)</summary><blockquote> `260-288`: **LGTM on Docker rebuild integration.** The conditional rebuild logic is correctly integrated with appropriate user feedback. The implementation mirrors start.mjs, ensuring consistent behavior between development and production modes. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
apps/ui/src/components/views/setup-view/hooks/use-cli-status.ts
Outdated
Show resolved
Hide resolved
| if [ -n "$CURSOR_AUTH_TOKEN" ]; then | ||
| CURSOR_CONFIG_DIR="/home/automaker/.config/cursor" | ||
| mkdir -p "$CURSOR_CONFIG_DIR" | ||
| # Write auth.json with the access token | ||
| cat > "$CURSOR_CONFIG_DIR/auth.json" << EOF | ||
| { | ||
| "accessToken": "$CURSOR_AUTH_TOKEN" | ||
| } | ||
| EOF | ||
| chmod 600 "$CURSOR_CONFIG_DIR/auth.json" | ||
| chown -R automaker:automaker /home/automaker/.config | ||
| fi |
There was a problem hiding this comment.
Token escaping issue remains unaddressed.
The CURSOR_AUTH_TOKEN is directly interpolated into the JSON heredoc without escaping. If the token contains ", \, or newlines, the resulting JSON will be malformed. This was flagged in a previous review.
🔎 Proposed fix using sed escaping
if [ -n "$CURSOR_AUTH_TOKEN" ]; then
CURSOR_CONFIG_DIR="/home/automaker/.config/cursor"
mkdir -p "$CURSOR_CONFIG_DIR"
+ # Escape special JSON characters in the token
+ ESCAPED_TOKEN=$(printf '%s' "$CURSOR_AUTH_TOKEN" | sed 's/[\"\\]/\\&/g' | tr -d '\n')
# Write auth.json with the access token
cat > "$CURSOR_CONFIG_DIR/auth.json" << EOF
{
- "accessToken": "$CURSOR_AUTH_TOKEN"
+ "accessToken": "$ESCAPED_TOKEN"
}
EOF
chmod 600 "$CURSOR_CONFIG_DIR/auth.json"
chown -R automaker:automaker /home/automaker/.config
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [ -n "$CURSOR_AUTH_TOKEN" ]; then | |
| CURSOR_CONFIG_DIR="/home/automaker/.config/cursor" | |
| mkdir -p "$CURSOR_CONFIG_DIR" | |
| # Write auth.json with the access token | |
| cat > "$CURSOR_CONFIG_DIR/auth.json" << EOF | |
| { | |
| "accessToken": "$CURSOR_AUTH_TOKEN" | |
| } | |
| EOF | |
| chmod 600 "$CURSOR_CONFIG_DIR/auth.json" | |
| chown -R automaker:automaker /home/automaker/.config | |
| fi | |
| if [ -n "$CURSOR_AUTH_TOKEN" ]; then | |
| CURSOR_CONFIG_DIR="/home/automaker/.config/cursor" | |
| mkdir -p "$CURSOR_CONFIG_DIR" | |
| # Escape special JSON characters in the token | |
| ESCAPED_TOKEN=$(printf '%s' "$CURSOR_AUTH_TOKEN" | sed 's/[\"\\]/\\&/g' | tr -d '\n') | |
| # Write auth.json with the access token | |
| cat > "$CURSOR_CONFIG_DIR/auth.json" << EOF | |
| { | |
| "accessToken": "$ESCAPED_TOKEN" | |
| } | |
| EOF | |
| chmod 600 "$CURSOR_CONFIG_DIR/auth.json" | |
| chown -R automaker:automaker /home/automaker/.config | |
| fi |
🤖 Prompt for AI Agents
In @docker-entrypoint.sh around lines 31-42, The current docker-entrypoint.sh
writes CURSOR_AUTH_TOKEN directly into the heredoc for
"$CURSOR_CONFIG_DIR/auth.json", which can produce invalid JSON if the token
contains quotes, backslashes, or newlines; instead serialize/escape the token
when producing the JSON (e.g., replace the heredoc write with a JSON-safe
serialization using jq or Python's json.dumps) so that CURSOR_AUTH_TOKEN is
properly escaped before writing to auth.json, and keep the mkdir, chmod 600 and
chown automaker:automaker steps for CURSOR_CONFIG_DIR.
- Eliminated redundant logger declaration within the useCliStatus hook to improve code clarity and prevent potential performance issues. - This change enhances the maintainability of the code by ensuring the logger is created only once outside the hook.
…functions - Removed redundant Docker image rebuilding logic from `dev.mjs` and `start.mjs`, centralizing it in the new `launchDockerContainers` function within `launcher-utils.mjs`. - Introduced `sanitizeProjectName` and `shouldRebuildDockerImages` functions to improve project name handling and Docker image management. - Updated the Docker launch process to provide clearer logging and ensure proper handling of environment variables, enhancing the overall development experience.
…Docker setup
These changes improve the development workflow and provide clear guidance on CLI installation and authentication processes.
Summary by CodeRabbit
New Features
Documentation
Chores
Tools
✏️ Tip: You can customize this high-level summary in your review settings.