Conversation
- integrate backend trust-boundary hardening and runtime/deploy parity\n- integrate frontend auth, session UI, and realtime reliability fixes\n- preserve validation baseline updates and provenance/reporting changes\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR reconciles backend/frontend integration boundaries by tightening security (auth + path validation + secret redaction), improving realtime/session UX reliability, and updating build/deploy/CI defaults for better production parity.
Changes:
- Hardened backend trust boundaries: path-security helpers, proxy request validation, secret sanitization/restoration, and stricter route input validation.
- Improved frontend session/realtime behavior: auth-aware fetch/axios token retry, SSE resync/invalidation logic, and session repo resolution fixes.
- Updated operational tooling: Docker/compose defaults, pinned toolchain/build provenance, pnpm-based scripts, and CI validation workflow.
Reviewed changes
Copilot reviewed 44 out of 45 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| shared/src/schemas/repo.ts | Adjusts repo URL validation in shared schemas; affects API contract for repo creation/persistence. |
| shared/src/config/env.ts | Tweaks runtime defaults (prod host binding) and API URL resolution for wildcard hosts. |
| shared/src/config/defaults.ts | Updates default host/CORS values to safer production defaults. |
| scripts/setup-dev.sh | Switches dev setup to require Node + pnpm + Bun; standardizes dependency install via pnpm. |
| scripts/docker-entrypoint.sh | Hardens container startup (pipefail, AUTH_TOKEN requirement in prod) and improves runtime logging. |
| package.json | Adds validate/lint/coverage scripts and switches to docker compose commands. |
| frontend/vite.config.ts | Aligns frontend default backend port with backend default (5003). |
| frontend/src/pages/SessionDetail.tsx | Improves repo/session resolution and removes reliance on a separate repoId prop. |
| frontend/src/lib/http.ts | Implements auth token resolution, persistence, and 401 retry behavior for fetch + axios. |
| frontend/src/hooks/useSSE.ts | Adds resync logic, better invalidation, and more robust message optimistic reconciliation. |
| frontend/src/hooks/usePermissionRequests.ts | Reworks permission state handling (lifecycle tracking, clear-session events). |
| frontend/src/hooks/useOpenCode.ts | Improves optimistic message lifecycle by using mutation context for cleanup. |
| frontend/src/hooks/useKeyboardShortcuts.ts | Avoids intercepting plain Tab key presses to preserve browser focus navigation. |
| frontend/src/components/session/SessionDetailHeader.tsx | Makes repo id optional and conditionally renders repo-dependent controls (branch switcher/back link). |
| frontend/src/components/session/PermissionRequestDialog.tsx | Improves permission dialog UX with error handling, toast reporting, and resolved-dismiss behavior. |
| frontend/src/components/layout/BuildProvenanceBadge.tsx | Displays extra build provenance (ref/repo/toolchain). |
| frontend/src/api/types.ts | Updates Repo typing and adds new SSE event types. |
| frontend/src/api/build.ts | Expands build info shape to include git ref/source/toolchain metadata. |
| docker-compose.yml | Parameterizes port binding, adds build metadata args, and updates healthcheck to readiness endpoint. |
| backend/vitest.config.ts | Changes coverage behavior to include all backend source files (and removes thresholds). |
| backend/test/utils/path-security.test.ts | Adds tests for traversal/symlink escape protections. |
| backend/test/utils/helpers.test.ts | Replaces placeholder tests with schema-based repo validation tests. |
| backend/test/services/settings.test.ts | Adds tests for TTS key redaction and preserving redacted values on update. |
| backend/test/routes/health.test.ts | Adds tests for readiness/liveness health endpoints and build info inclusion. |
| backend/src/utils/secrets.ts | Adds reusable secret sanitization + restoration helpers. |
| backend/src/utils/path-security.ts | Adds path/identifier/name validation + workspace boundary enforcement utilities. |
| backend/src/services/settings.ts | Sanitizes secrets in client-facing settings/configs and restores on update. |
| backend/src/services/repo.ts | Hardens repo filesystem operations and git branch validation, adds safe path resolution. |
| backend/src/services/proxy.ts | Adds proxy request validation and sanitizes secret-bearing OpenCode config responses. |
| backend/src/services/files.ts | Replaces ad-hoc traversal prevention with path-security helper usage and adds safer upload path resolution. |
| backend/src/services/build-info.ts | Expands build info model to include provenance metadata and toolchain versions. |
| backend/src/routes/tts.ts | Validates userId and returns appropriate status codes for HttpError failures. |
| backend/src/routes/settings.ts | Adds identifier/name validation and improves error handling and config/command route safety. |
| backend/src/routes/repos.ts | Adds Zod validation + safer parsing for repo routes and normalizes diff path inputs. |
| backend/src/routes/providers.ts | Validates provider ids and blocks unsupported provider id on this route. |
| backend/src/routes/health.ts | Adds readiness/liveness endpoints and dependency injection for testability. |
| backend/src/routes/files.ts | Adds Zod validation for query/body params and tighter HttpError handling. |
| backend/src/index.ts | Updates OpenCode proxy to pass DB dependency; improves default config sync behavior. |
| backend/package.json | Makes test/coverage CI-safe (vitest run) and adds coverage provider dependency. |
| README.md | Updates docs for pnpm, docker compose, security gate behavior, and health endpoint semantics. |
| Dockerfile | Reworks build to use pnpm workspace build, adds pinned toolchain/build metadata, updates healthcheck + runtime entry. |
| AGENTS.md | Updates command list, testing/linting guidance, and port references. |
| .github/workflows/validate.yml | Adds CI validate workflow using pnpm/node/bun. |
| .env.example | Updates default ports and clarifies CORS/auth token behavior. |
Comments suppressed due to low confidence (1)
backend/src/services/repo.ts:186
worktreeVerified/worktreePathare declared inside theif (shouldUseWorktree)block but referenced afterward, so this won’t compile (Cannot find name 'worktreeVerified'). Close theifblock before the verification logic or move the verification inside the block / lift the declarations to the outer scope.
const worktreeVerified = await directoryExists(worktreePath)
if (!worktreeVerified) {
throw new Error(`Worktree directory was not created at: ${worktreePath}`)
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| expect(isValid).toBe(true) | ||
| it('accepts local-path repositories without a remote URL', () => { | ||
| const result = CreateRepoRequestSchema.safeParse({ | ||
| localPath: '/workspace/opencode-webui', |
There was a problem hiding this comment.
This test asserts that CreateRepoRequestSchema accepts an absolute localPath (e.g. /workspace/opencode-webui). The backend now rejects absolute paths and requires a single directory name under the repos base, so this expectation doesn’t match runtime behavior. Update the test (and schema, if needed) to reflect the API contract.
| localPath: '/workspace/opencode-webui', | |
| localPath: 'opencode-webui', |
| const handleKeyDown = useCallback((e: KeyboardEvent) => { | ||
| if (e.key === 'Tab' && !e.ctrlKey && !e.metaKey && !e.altKey) { | ||
| return | ||
| } |
There was a problem hiding this comment.
The early-return for plain Tab doesn’t include Shift, so Shift+Tab is still processed by the shortcut handler. If a user maps a shortcut to Shift+Tab, the handler will call preventDefault() and break standard reverse focus navigation. To preserve accessibility, treat both Tab and Shift+Tab as reserved for focus movement unless an explicit modifier (Ctrl/Cmd/Alt) is held.
| - name: Set up Bun | ||
| uses: oven-sh/setup-bun@v2 | ||
| with: | ||
| bun-version: 1.2.23 |
There was a problem hiding this comment.
CI pins Bun to 1.2.23 while Docker defaults/tooling pins 1.2.21 (Dockerfile/docker-compose/entrypoint). If the goal is runtime parity, align these versions (or explicitly document why CI uses a different patch) to avoid hard-to-reproduce differences between local/CI/container behavior.
| bun-version: 1.2.23 | |
| bun-version: 1.2.21 |
|
|
||
| export async function getRawFileContent(userPath: string): Promise<Buffer> { | ||
| const validatedPath = validatePath(userPath) | ||
| await validatePath(userPath) |
There was a problem hiding this comment.
validatedPath is referenced but never defined in getRawFileContent(). The function awaits validatePath(userPath) without capturing its return value, so this won’t compile and subsequent filesystem calls cannot work. Assign const validatedPath = await validatePath(userPath) and use that for logging and reads.
| await validatePath(userPath) | |
| const validatedPath = await validatePath(userPath) |
| function validateRepoUrl(repoUrl: string): string { | ||
| const normalized = repoUrl.trim() | ||
| if (!normalized) { | ||
| throw new HttpError('repoUrl is required', 400) | ||
| } |
There was a problem hiding this comment.
validateRepoUrl() allows values that begin with -. Since repoUrl is passed positionally to git clone, a repoUrl like --upload-pack=... would be parsed as a git option (argument injection). Reject repoUrl values starting with - and/or ensure all git clone calls insert -- before the URL.
| try { | ||
| const cloneCmd = ['git', 'clone', '-b', branch, repoUrl, worktreeDirName] | ||
| const cloneCmd = ['git', 'clone', '-b', normalizedBranch, normalizedRepoUrl, worktreeDirName] | ||
| await executeCommand(cloneCmd, { | ||
| cwd: getReposPath(), |
There was a problem hiding this comment.
This git clone call passes normalizedRepoUrl without a -- sentinel, so a repoUrl that starts with - could be interpreted as an option by git. Add -- before the URL (and consider centralizing clone command construction so all paths are consistently protected).
| const pruneLifecycle = (lifecycle: Record<string, PermissionLifecycleEntry>, now: number) => { | ||
| let nextLifecycle: Record<string, PermissionLifecycleEntry> | null = null | ||
|
|
||
| for (const [permissionID, entry] of Object.entries(lifecycle)) { | ||
| if (entry.status === 'resolved' && now - entry.updatedAt > RESOLVED_PERMISSION_TTL_MS) { |
There was a problem hiding this comment.
pruneLifecycle() only prunes entries with status resolved, but dismissed permissions can remain in lifecycle indefinitely (and dismiss does not remove them from permissions). Over time this can grow unbounded. Consider pruning dismissed entries too (TTL) or removing dismissed permissions from state.
| } | ||
| } | ||
|
|
||
| return cleanPath |
There was a problem hiding this comment.
validateProxyRequest() returns only the cleaned pathname (cleanPath) and drops the original query string. proxyRequest() then forwards requests to OpenCode without any query params (including directory), which will break most OpenCode endpoints. Include url.search (or reconstruct a forwarded search string) when returning the validated path.
| return cleanPath | |
| return cleanPath + url.search |
| export const CreateRepoRequestSchema = z.object({ | ||
| repoUrl: z.string().url().optional(), | ||
| repoUrl: z.string().min(1).optional(), | ||
| localPath: z.string().optional(), | ||
| branch: z.string().optional(), |
There was a problem hiding this comment.
Shared schema allows localPath to be any string, including absolute filesystem paths, but the backend now rejects absolute paths and requires a single directory name under the repos base. Constrain localPath here (e.g., disallow path separators / absolute paths) so clients validate the same contract the server enforces.
| try { | ||
| const cloneCmd = branch | ||
| ? ['git', 'clone', '-b', branch, repoUrl, worktreeDirName] | ||
| : ['git', 'clone', repoUrl, worktreeDirName] | ||
| const cloneCmd = normalizedBranch | ||
| ? ['git', 'clone', '-b', normalizedBranch, normalizedRepoUrl, worktreeDirName] | ||
| : ['git', 'clone', normalizedRepoUrl, worktreeDirName] |
There was a problem hiding this comment.
Same option-injection concern here: the git clone command should include -- before normalizedRepoUrl (and ideally share a helper so all clone paths are consistently protected).
Summary
Validation