Skip to content

fix(shell): restart browser sidecar reliably via healthcheck + DinD isolation#157

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
konard wants to merge 17 commits into
base:main
Choose a base branch
from
Open

fix(shell): restart browser sidecar reliably via healthcheck + DinD isolation #157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
924b7fa
fix(shell): restart browser sidecar reliably via healthcheck + DinD i…
skulidropek Mar 15, 2026
9dda672
feat(api): add auth/state/scrap/sessions/mcp-playwright endpoints
skulidropek Mar 16, 2026
c9531d0
feat(app): add api-client HTTP module
skulidropek Mar 16, 2026
d91380a
feat(app): rewrite CLI program to use unified REST API
skulidropek Mar 16, 2026
c0cef75
fix(api): change auth status routes from GET to POST
skulidropek Mar 16, 2026
a7d99b6
feat(ssh): enable password auth out of the box
skulidropek Mar 16, 2026
ccb578e
feat(ssh): embed password in sshCommand via sshpass
skulidropek Mar 16, 2026
48e563e
Merge remote-tracking branch 'origin/main' into issue-137
skulidropek Mar 16, 2026
35fd964
sync
skulidropek Mar 16, 2026
2d75759
fix: merge main and resolve conflicts, preserve sshpass support
konard Mar 18, 2026
bbd6655
fix(lint): resolve CI failures — remove dead api-client, fix Effect-T…
konard Mar 18, 2026
593f3f1
fix(app): revert main.ts to upstream — FetchHttpClient no longer needed
konard Mar 18, 2026
9eae39c
Merge remote-tracking branch 'upstream/main' into issue-137
konard Mar 18, 2026
e97a214
Merge remote-tracking branch 'upstream/main' into issue-137
konard Mar 18, 2026
c0fa1cd
fix(shell): resolve merge conflict — keep healthcheck + add upstream …
konard Mar 23, 2026
02bd091
fix(lint): apply import ordering and formatting fixes
konard Mar 23, 2026
486ae9c
Merge upstream/main (version packages)
konard Mar 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions Dockerfile
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,14 +15,17 @@ RUN useradd -m -s /bin/bash dev
# sshd runtime dir
RUN mkdir -p /run/sshd

# Harden sshd: disable password auth and root login
# sshd: password auth enabled so users can connect without key setup
RUN printf "%s\n" \
"PasswordAuthentication no" \
"PasswordAuthentication yes" \
"PermitRootLogin no" \
"PubkeyAuthentication yes" \
"AllowUsers dev" \
> /etc/ssh/sshd_config.d/dev.conf

# Default password = username (works out of the box; key auth still accepted if authorized_keys provided)
RUN echo "dev:dev" | chpasswd

# Workspace in dev home
RUN mkdir -p /home/dev/app && chown -R dev:dev /home/dev

Expand Down
17 changes: 16 additions & 1 deletion docker-compose.api.yml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,14 @@
services:
dind:
image: docker:27-dind
container_name: docker-git-dind
privileged: true
environment:
DOCKER_TLS_CERTDIR: ""
volumes:
- docker-git-dind-storage:/var/lib/docker
restart: unless-stopped
Comment on lines +2 to +10

@coderabbitaicoderabbitaiBotMay 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy lift

Critical security violation: privileged container with disabled TLS.

The DinD service configuration violates security best practices:

  1. privileged: true (line 5): Grants the container full access to host devices and kernel capabilities, bypassing Docker's isolation model. Any compromise of this container grants root-equivalent access to the host.

  2. DOCKER_TLS_CERTDIR: "" (line 7): Disables TLS encryption and authentication for the Docker daemon.

  3. DOCKER_HOST: tcp://dind:2375 (line 22): Uses unencrypted TCP connection without authentication. Any container on the same network can execute arbitrary Docker commands on this daemon.

Combined risk: An attacker who compromises the api container or any container on the same network can control the DinD daemon without authentication, create privileged containers, and escape to the host system.

Recommended alternatives:

  • Use Docker socket proxy (e.g., tecnativa/docker-socket-proxy) with minimal permissions instead of full DinD
  • If DinD is absolutely required, enable TLS (DOCKER_TLS_CERTDIR=/certs) and configure mTLS between api and dind
  • Consider whether the API actually needs Docker daemon access at all

As per coding guidelines: "Reject PRs with unsafe Docker/GitHub Actions configuration such as privileged containers, broad host mounts, unbounded Docker socket access."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker-compose.api.yml` around lines 2 - 10, The dind service currently runs
privileged with DOCKER_TLS_CERTDIR="" and the stack sets DOCKER_HOST to
tcp://dind:2375, creating an unprotected Docker daemon; update the
docker-compose to remove privileged: true from the dind service, set
DOCKER_TLS_CERTDIR to a non-empty path (e.g., /certs) and configure TLS
certificates for mTLS between clients and the daemon, or better yet replace the
dind service with a restricted docker socket proxy (e.g.,
tecnativa/docker-socket-proxy) and update the api service to point to the proxy
socket instead of tcp://dind:2375 (or eliminate Docker access entirely if not
required); ensure any references to DOCKER_HOST in your compose/env now use a
secure socket or tls-enabled endpoint and revoke broad network access to the
daemon.


api:
build:
context: .
Expand All@@ -9,9 +19,14 @@ services:
DOCKER_GIT_PROJECTS_ROOT: ${DOCKER_GIT_PROJECTS_ROOT:-/home/dev/.docker-git}
DOCKER_GIT_FEDERATION_PUBLIC_ORIGIN: ${DOCKER_GIT_FEDERATION_PUBLIC_ORIGIN:-}
DOCKER_GIT_FEDERATION_ACTOR: ${DOCKER_GIT_FEDERATION_ACTOR:-docker-git}
DOCKER_HOST: tcp://dind:2375
ports:
- "${DOCKER_GIT_API_BIND_HOST:-127.0.0.1}:${DOCKER_GIT_API_PORT:-3334}:${DOCKER_GIT_API_PORT:-3334}"
volumes:
- /var/run/docker.sock:/var/run/docker.sock
- ${DOCKER_GIT_PROJECTS_ROOT_HOST:-/home/dev/.docker-git}:${DOCKER_GIT_PROJECTS_ROOT:-/home/dev/.docker-git}
depends_on:
- dind
restart: unless-stopped

volumes:
docker-git-dind-storage:
115 changes: 115 additions & 0 deletions packages/api/src/api/contracts.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -227,3 +227,118 @@ export type ApiEvent = {
readonly at: string
readonly payload: unknown
}

// Auth
export type AuthStatusResponse = { readonly message: string }

export type AuthGithubLoginRequest = {
readonly label?: string | null | undefined
readonly token?: string | null | undefined
readonly scopes?: string | null | undefined
readonly envGlobalPath: string
}

export type AuthGithubStatusRequest = {
readonly envGlobalPath: string
}

export type AuthGithubLogoutRequest = {
readonly label?: string | null | undefined
readonly envGlobalPath: string
}

export type AuthCodexLoginRequest = {
readonly label?: string | null | undefined
readonly codexAuthPath: string
}

export type AuthCodexStatusRequest = {
readonly label?: string | null | undefined
readonly codexAuthPath: string
}

export type AuthCodexLogoutRequest = {
readonly label?: string | null | undefined
readonly codexAuthPath: string
}

export type AuthClaudeLoginRequest = {
readonly label?: string | null | undefined
readonly claudeAuthPath: string
}

export type AuthClaudeStatusRequest = {
readonly label?: string | null | undefined
readonly claudeAuthPath: string
}

export type AuthClaudeLogoutRequest = {
readonly label?: string | null | undefined
readonly claudeAuthPath: string
}

// State
export type StateInitRequest = {
readonly repoUrl: string
readonly repoRef?: string | undefined
}

export type StateCommitRequest = {
readonly message: string
}

export type StateSyncRequest = {
readonly message?: string | null | undefined
}

export type StatePathResponse = { readonly path: string }

export type StateOutputResponse = { readonly output: string }

// Scrap
export type ScrapExportRequest = {
readonly projectDir: string
readonly archivePath?: string | undefined
}

export type ScrapImportRequest = {
readonly projectDir: string
readonly archivePath: string
readonly wipe?: boolean | undefined
}

// Sessions
export type SessionsListRequest = {
readonly projectDir: string
readonly includeDefault?: boolean | undefined
}

export type SessionsKillRequest = {
readonly projectDir: string
readonly pid: number
}

export type SessionsLogsRequest = {
readonly projectDir: string
readonly pid: number
readonly lines?: number | undefined
}

export type SessionsOutput = { readonly output: string }

// MCP Playwright
export type McpPlaywrightUpRequest = {
readonly projectDir: string
readonly runUp?: boolean | undefined
}

// Apply (project config)
export type ApplyRequest = {
readonly runUp?: boolean | undefined
readonly gitTokenLabel?: string | undefined
readonly codexTokenLabel?: string | undefined
readonly claudeTokenLabel?: string | undefined
readonly cpuLimit?: string | undefined
readonly ramLimit?: string | undefined
readonly enableMcpPlaywright?: boolean | undefined
}
101 changes: 101 additions & 0 deletions packages/api/src/api/schema.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -86,3 +86,104 @@ export const AgentLogLineSchema = Schema.Struct({
export type CreateProjectRequestInput = Schema.Schema.Type<typeof CreateProjectRequestSchema>
export type CreateAgentRequestInput = Schema.Schema.Type<typeof CreateAgentRequestSchema>
export type CreateFollowRequestInput = Schema.Schema.Type<typeof CreateFollowRequestSchema>

export const AuthGithubLoginRequestSchema = Schema.Struct({
label: Schema.optional(Schema.NullOr(Schema.String)),
token: Schema.optional(Schema.NullOr(Schema.String)),
scopes: Schema.optional(Schema.NullOr(Schema.String)),
envGlobalPath: Schema.String
})

export const AuthGithubStatusRequestSchema = Schema.Struct({
envGlobalPath: Schema.String
})

export const AuthGithubLogoutRequestSchema = Schema.Struct({
label: Schema.optional(Schema.NullOr(Schema.String)),
envGlobalPath: Schema.String
})

export const AuthCodexLoginRequestSchema = Schema.Struct({
label: Schema.optional(Schema.NullOr(Schema.String)),
codexAuthPath: Schema.String
})

export const AuthCodexStatusRequestSchema = Schema.Struct({
label: Schema.optional(Schema.NullOr(Schema.String)),
codexAuthPath: Schema.String
})

export const AuthCodexLogoutRequestSchema = Schema.Struct({
label: Schema.optional(Schema.NullOr(Schema.String)),
codexAuthPath: Schema.String
})

export const AuthClaudeLoginRequestSchema = Schema.Struct({
label: Schema.optional(Schema.NullOr(Schema.String)),
claudeAuthPath: Schema.String
})

export const AuthClaudeStatusRequestSchema = Schema.Struct({
label: Schema.optional(Schema.NullOr(Schema.String)),
claudeAuthPath: Schema.String
})

export const AuthClaudeLogoutRequestSchema = Schema.Struct({
label: Schema.optional(Schema.NullOr(Schema.String)),
claudeAuthPath: Schema.String
})

export const StateInitRequestSchema = Schema.Struct({
repoUrl: Schema.String,
repoRef: OptionalString
})

export const StateCommitRequestSchema = Schema.Struct({
message: Schema.String
})

export const StateSyncRequestSchema = Schema.Struct({
message: Schema.optional(Schema.NullOr(Schema.String))
})

export const ScrapExportRequestSchema = Schema.Struct({
projectDir: Schema.String,
archivePath: OptionalString
})

export const ScrapImportRequestSchema = Schema.Struct({
projectDir: Schema.String,
archivePath: Schema.String,
wipe: OptionalBoolean
})

export const SessionsListRequestSchema = Schema.Struct({
projectDir: Schema.String,
includeDefault: OptionalBoolean
})

export const SessionsKillRequestSchema = Schema.Struct({
projectDir: Schema.String,
pid: Schema.Number
})

export const SessionsLogsRequestSchema = Schema.Struct({
projectDir: Schema.String,
pid: Schema.Number,
lines: Schema.optional(Schema.Number)
Comment on lines +165 to +173

@coderabbitaicoderabbitaiBotMay 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Constrain pid/lines to safe positive integers.

pid and lines accept any number right now (including negatives/decimals). For process control/log tailing endpoints, this should be validated as positive integers (and lines should have a sane upper bound).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/api/src/api/schema.ts` around lines 165 - 173, The pid and lines
fields currently accept any number; update SessionsKillRequestSchema and
SessionsLogsRequestSchema so pid is a positive integer (use the integer
validation from Schema, e.g., Schema.Int with a >0 constraint) and lines is an
optional positive integer with a sane upper bound (e.g., min 1 and max like
1000) to prevent negatives/decimals/unbounded values; adjust the validations on
SessionsKillRequestSchema.pid and SessionsLogsRequestSchema.pid/lines
accordingly so they reject non-integers, zero, and negative numbers and enforce
the lines cap.

})

export const McpPlaywrightUpRequestSchema = Schema.Struct({
projectDir: Schema.String,
runUp: OptionalBoolean
})
Comment on lines +90 to +179

@coderabbitaicoderabbitaiBotMay 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Reject empty strings for required path-like inputs.

Several required filesystem/config path fields are plain strings, so "" currently validates. This defers invalid input to deeper layers and turns a request validation concern into runtime failures.

Please tighten these fields to non-empty trimmed strings at the schema boundary.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/api/src/api/schema.ts` around lines 90 - 179, Several required
path-like string fields currently accept empty strings; create a shared
validated type (e.g., NonEmptyTrimmedString) that trims input and rejects
zero-length values, then replace Schema.String with that helper for all
path-like required fields: envGlobalPath in
AuthGithubLoginRequestSchema/AuthGithubStatusRequestSchema/AuthGithubLogoutRequestSchema,
codexAuthPath in
AuthCodexLoginRequestSchema/AuthCodexStatusRequestSchema/AuthCodexLogoutRequestSchema,
claudeAuthPath in
AuthClaudeLoginRequestSchema/AuthClaudeStatusRequestSchema/AuthClaudeLogoutRequestSchema,
repoUrl in StateInitRequestSchema, projectDir in
ScrapExportRequestSchema/ScrapImportRequestSchema/SessionsListRequestSchema/SessionsKillRequestSchema/SessionsLogsRequestSchema/McpPlaywrightUpRequestSchema,
and archivePath in ScrapExportRequestSchema/ScrapImportRequestSchema; leave
optional/nullable helpers (OptionalString/OptionalBoolean) unchanged. Ensure the
helper trims input and fails validation for empty strings so invalid requests
are rejected at the schema boundary.


export const ApplyRequestSchema = Schema.Struct({
runUp: OptionalBoolean,
gitTokenLabel: OptionalString,
codexTokenLabel: OptionalString,
claudeTokenLabel: OptionalString,
cpuLimit: OptionalString,
ramLimit: OptionalString,
enableMcpPlaywright: OptionalBoolean
})
Loading