fix: harden proxy error handling, worktree CLI, and config validation - #73
Open
reopt-eric wants to merge 6 commits into
Open
fix: harden proxy error handling, worktree CLI, and config validation#73reopt-eric wants to merge 6 commits into
reopt-eric wants to merge 6 commits into
Conversation
A single request could surface a terminal event from multiple sources (proxyRes error, proxyReq error, normal completion), firing a second writeHead/end and throwing ERR_HTTP_HEADERS_SENT. Guard with a settled flag so only the first one emits and touches clientRes. Also destroy the upstream stream when the client aborts mid-response, escape single quotes in error pages, and strip hop-by-hop headers (RFC 7230) before relaying. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A request for an unregistered worktree service silently fell back to another service's first port, masking misconfiguration and risking routing to the wrong app. Return null for a named-but-absent service (per the no-silent-fallback routing invariant); only default to the first port when no service is given. Also validate that port values are finite positive integers so a malformed instance file can't break routing with an invalid URL. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
worktree destroy skipped the isValidSubdomain guard that create applies,
letting an unvalidated branch flow into {branch} directory substitution
and the post-remove hook cwd (path traversal). Add the guard before any
resolution. Switch git calls to execFileSync to remove the shell, pass
only { worktrees } to writeProjectConfig so a legacy .dev-proxy.json
isn't resurrected, re-read worktrees before writing to narrow the port
TOCTOU window, and prefer the longest matching prefix in findOwningProject.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
doctor read worktreeConfig and routes from readProjectConfig, which only parses JSON — so for mjs-based projects (now the standard) the value was always undefined and the entire portRange/services/directory validation block was silently skipped. Source worktreeConfig and routes from the resolved ProjectConfig singleton instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When a request started while detail collection was active but its request:complete arrived after the 30s idle timeout, the response headers were dropped and the detail panel showed (awaiting response) forever. Collect detail when active OR when the id already holds a detail entry, so an in-flight request's response is merged regardless of idle state (still bounded by the LRU). Also invalidate the filtered cache explicitly after in-place eviction. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Treat --help/--version as global only in the command position so a subcommand argument or flag isn't hijacked. Surface a cleanup-failed status when migrate cannot delete the legacy .dev-proxy.json (it was swallowed silently, leaving config precedence ambiguous). Reject port input with trailing garbage in init by parsing with Number instead of parseInt. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
개요
전체 프로젝트 리뷰에서 발견한 문제들을 영역별로 수정했습니다. 모든 변경에 회귀 테스트를 추가했고, 적대적 리뷰로 새 버그가 없음을 교차 검증했습니다.
변경 내용
🔴 High
server.ts):proxyRes/proxyReqerror와 정상 완료가 동시에clientRes를 건드려 발생하던ERR_HTTP_HEADERS_SENT크래시를settled플래그로 차단. 클라이언트 중단 시proxyRes.destroy()로 소켓 누수 방지, hop-by-hop 헤더 스트립(RFC 7230), 에러 페이지'이스케이프.worktree.tsx):destroy에isValidSubdomain가드 추가(경로 탈출·hook cwd 차단), git 호출을execFileSync로 전환(셸 제거), 포트 할당 TOCTOU 재읽기+충돌 감지,{ worktrees }만 기록해 레거시.dev-proxy.json부활 방지.doctor.tsx):worktreeConfig/routes를 resolved 싱글톤에서 읽도록 변경 — mjs 프로젝트(현재 표준)에서 통째로 스킵되던 검증을 복구.🟠 Medium
store.ts): active→idle 전환 후 완료된 요청의 응답 헤더가 영구 누락되던 문제를detailMap.has(id)면 idle이어도 merge하도록 수정(LRU 한도 유지). eviction 후 필터 캐시 명시적 무효화.worktrees.ts,config-io.ts): 미등록 service는null반환(silent fallback 제거), port 값 검증.🟡 Low
cli.ts: 전역 플래그를 command 위치에서만 처리 → 하위 인자 hijack 방지migrate.tsx: 레거시 파일 삭제 실패를cleanup-failed로 노출init.tsx: 포트 파싱을Number로 통일("4000abc"거부)worktree.tsx:findOwningProject경로 정규화 + 최장 prefix 우선테스트
보류
selectByFilteredIndex가 bounds-check를 하므로 영향이 eviction 시의 드문 edge에 한정. UI는 수동 검증 영역이라 별도 작업으로 분리.🤖 Generated with Claude Code