Skip to content

fix: gracefully disable host-specific commands in Docker mode (#24)#27

Merged
dreamwing merged 9 commits intomasterfrom
fix/issue-24-docker-execution
Mar 13, 2026
Merged

fix: gracefully disable host-specific commands in Docker mode (#24)#27
dreamwing merged 9 commits intomasterfrom
fix/issue-24-docker-execution

Conversation

@dreamwing
Copy link
Owner

This PR fixes the issue where Clawbridge inside Docker attempts to execute host-level processes (like openclaw, ps, pgrep, and pkill), resulting in errors and log pollution.

Changes

  • Introduces IS_DOCKER check in config.js.
  • monitor.js: Skips process scanning and openclaw version CLI fetching when containerized.
  • process.js: Returns 403 JSON errors with clear instructions if a user attempts to kill or restart processes from the UI in Docker mode.
  • cron.js: Disables manual cron job execution and CLI list fallback.

Closes #24

- Detect Docker context via /.dockerenv
- Skip openclaw CLI presence/version checks in Docker
- Suppress gateway process scanning (ps/pgrep) in monitor to prevent log pollution
- Gracefully disable /api/kill, /api/gateway/restart, and /api/run/:id endpoints with clear error responses (Fixes #24 host execution issue)
@dreamwing
Copy link
Owner Author

@greptileai review please~

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 13, 2026

Greptile Summary

This PR introduces comprehensive Docker-awareness across Clawbridge, gracefully disabling or stubbing out every host-scoped operation when the service is running in a container. All previously identified issues (IS_DOCKER guard ordering, strict env-var comparison, disk metric false-reporting, scripts null consistency, unnecessary df subprocess, and the dockerMode cron marker) have been addressed in prior commits. Two minor style observations remain.

  • src/config.js: IS_DOCKER detection is robust — combines /.dockerenv file presence with a case-insensitive env-var check accepting 'true', '1', and 'yes'.
  • src/services/monitor.js: The refactored finalizeStatus helper cleanly separates the Docker and non-Docker execution paths; the exec subprocess is skipped entirely in Docker mode. Minor: finalizeStatus is defined as an inner closure on every checkSystemStatus invocation — moving it to module scope with callback as an explicit parameter would avoid the per-call allocation.
  • src/routes/process.js and src/routes/cron.js: All Docker guards are placed correctly before any request validation or rate-limiting.
  • public/js/dashboard.js: fetchJobs correctly handles the { dockerMode: true, jobs: [] } response and all action handlers now surface API errors via alert. Minor: container.innerHTML = '' fires before the dockerMode check — the check should be hoisted above it to avoid the unnecessary DOM mutation.
  • Test coverage is solid: both new test files use module-level mocking with jest.doMock/jest.resetModules to achieve proper isolation, and all four route guards plus the two monitor behaviours are exercised.

Confidence Score: 4/5

  • This PR is safe to merge; all host-scoped operations are correctly guarded and previously flagged issues have been resolved.
  • The implementation is functionally correct and well-tested. All issues raised in prior review threads have been addressed. The two remaining observations are minor style improvements (DOM mutation ordering in fetchJobs and inner-function allocation in monitor.js) that do not affect correctness or security. A score of 4 rather than 5 reflects these small polish items.
  • No files require special attention; src/services/monitor.js and public/js/dashboard.js have the two minor style points noted in inline comments.

Important Files Changed

Filename Overview
src/config.js Introduces IS_DOCKER flag using /.dockerenv file detection and a normalized env-var check accepting 'true', '1', and 'yes' (case-insensitive). Clean and robust.
src/routes/process.js Both /api/kill and /api/gateway/restart place the IS_DOCKER guard at the very top, before body validation and rate-limiting respectively. All previously flagged ordering issues have been resolved.
src/routes/cron.js Docker guard correctly blocks the CLI fallback and /api/run/:id. The fast path (state-file read) intentionally runs before the Docker guard, allowing volume-mounted state files to be served in Docker. POST /api/run/:id IS_DOCKER check correctly precedes ID validation.
src/services/monitor.js Solid refactor extracting finalizeStatus to handle both Docker and non-Docker paths. Docker mode skips the exec subprocess entirely. All five host-scoped metrics (cpu, mem, disk, gatewayPid, scripts) return null with the unsupportedMonitoring list. Minor: finalizeStatus is defined as an inner closure, recreated on every call.
public/js/dashboard.js Helper utilities (isUnsupportedMetric, readErrorMessage, setMetricValue) are clean additions. fetchJobs correctly handles the { dockerMode: true, jobs: [] } response. All action handlers now surface API errors via alert. Minor ordering issue: container.innerHTML = '' fires before the dockerMode check in fetchJobs.
public/index.html Minimal change: adds id="runtime-env" to the Environment span so dashboard.js can update it dynamically. Trailing newline also fixed.
tests/routes.docker-mode.test.js New test file with four focused unit tests covering all four Docker-mode route guards using jest.doMock isolation. Validates both status codes and response bodies. The invokeRouteHandler helper correctly bypasses Express middleware for direct handler testing.
tests/services.monitor.test.js Two well-structured tests: one asserts execSync is not called for version detection in Docker mode, the other asserts exec is not called and all host metrics are null with correct unsupportedMonitoring list.

Sequence Diagram

sequenceDiagram
    participant Client as Browser (dashboard.js)
    participant Config as config.js (IS_DOCKER)
    participant Status as GET /api/status
    participant Cron as GET /api/cron
    participant Run as POST /api/run/:id
    participant Kill as POST /api/kill
    participant Restart as POST /api/gateway/restart
    participant Monitor as monitor.js
    participant Exec as Child Process (exec/pgrep/ps)

    Note over Config: IS_DOCKER = /.dockerenv exists<br/>OR env IS_DOCKER in [true,1,yes]

    Client->>Status: fetchStatus()
    Status->>Monitor: checkSystemStatus(callback)
    alt IS_DOCKER = true
        Monitor-->>Status: finalizeStatus({}) [sync]<br/>cpu/mem/disk/gatewayPid/scripts = null<br/>unsupportedMonitoring = [all 5]
    else IS_DOCKER = false
        Monitor->>Exec: exec(df + pgrep + ps)
        Exec-->>Monitor: stdout sections
        Monitor-->>Status: finalizeStatus({diskUsage, gatewayPid, psOutput})
    end
    Status-->>Client: {cpu, mem, disk, gatewayPid, scripts,<br/>environment: {isDocker}, unsupportedMonitoring}
    Client->>Client: render "N/A" for unsupported metrics<br/>update runtime-env label

    Client->>Cron: fetchJobs()
    Cron->>Cron: try fast path (state file)
    alt state file readable
        Cron-->>Client: [jobs array] (raw, no dockerMode flag)
        Client->>Client: render jobs + run buttons
    else state file missing/error AND IS_DOCKER
        Cron-->>Client: {dockerMode: true, jobs: []}
        Client->>Client: render "Unavailable in Docker Mode"
    else state file missing AND not Docker
        Cron->>Exec: exec(openclaw cron list --json)
        Exec-->>Cron: stdout
        Cron-->>Client: [jobs array]
    end

    Client->>Run: runJob(id) POST /api/run/:id
    alt IS_DOCKER
        Run-->>Client: 403 {error: "...Docker Mode..."}
        Client->>Client: alert(error message)
    else
        Run->>Exec: execFile(openclaw cron run id)
        Exec-->>Run: result
        Run-->>Client: {status: triggered}
    end

    Client->>Kill: killAll() POST /api/kill {confirm:true}
    alt IS_DOCKER
        Kill-->>Client: 403 {error: "...Docker..."}
    else
        Kill->>Exec: pgrep + SIGTERM/SIGKILL
        Kill-->>Client: {status: stopping, pids: [...]}
    end

    Client->>Restart: restartGateway() POST /api/gateway/restart
    alt IS_DOCKER
        Restart-->>Client: 403 {error: "...Docker containers..."}
    else
        Restart->>Exec: pkill + openclaw gateway start
        Restart-->>Client: {status: restarted}
    end
Loading

Comments Outside Diff (1)

  1. public/js/dashboard.js, line 370-376 (link)

    dockerMode check fires after DOM is already cleared

    container.innerHTML = '' always runs before the dockerMode guard. In Docker mode (where data.jobs is always []), this means the container is blanked and then immediately rewritten — an unnecessary DOM mutation on every poll interval. Moving the Docker check above the clear would be cleaner and avoid the extra write:

Last reviewed commit: db7c4d6

dreamwing and others added 2 commits March 13, 2026 22:34
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@dreamwing
Copy link
Owner Author

Follow-up addressed in db7c4d6.

Changes made:

  • POST /api/run/:id now checks IS_DOCKER before ID validation, so Docker mode consistently returns 403 regardless of input.
  • checkSystemStatus() now short-circuits the Docker path without spawning a child process, while preserving checkFileChanges(), active-context detection, and activity logging logic.
  • Tests were updated to cover both behaviors.

Verified with:

  • npx jest tests/services.monitor.test.js tests/routes.docker-mode.test.js --runInBand
  • npx eslint src/routes/cron.js src/services/monitor.js tests/services.monitor.test.js tests/routes.docker-mode.test.js

@dreamwing dreamwing merged commit db7c4d6 into master Mar 13, 2026
5 checks passed
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.

Feature Request: versioned docker images

1 participant