fix(security): isolate web E2E commands and readiness probes

CHANGELOG.md

@@ -5,6 +5,41 @@ this file. The format follows Keep a Changelog, and versioned releases follow
 Semantic Versioning where the repository publishes a release.
 
 ## [Unreleased]
+- Web verification now runs backend, frontend, and E2E commands inside an
+  isolated Linux bubblewrap workspace by default (`--isolation required`),
+  mounting a read-only runtime root with a single writable `/workspace`
+  bind; trusted local debugging may opt out with `--isolation disabled`.
+  Isolation-backend resolution and the existing loopback readiness-URL
+  boundary are now both checked before any service starts, so an
+  unavailable isolation backend or an invalid readiness URL fails closed
+  with a clear diagnostic (exit code 126/125) instead of after services are
+  already running.
+- Close four gaps a Devin Review pass found in the same web E2E isolation
+  helper (`scripts/ci/sandboxed_web_e2e.py`, `scripts/ci/sandboxed_verify.py`):
+  a non-numeric or out-of-range readiness-URL port now raises the same
+  `ValueError` every other readiness check raises, instead of an uncaught
+  `http.client.InvalidURL` escaping past `main`'s exit-125 handling; a `bwrap`
+  binary on `PATH` now passes a bounded capability preflight (proving it can
+  actually create the sandbox's namespaces) before isolation is trusted as
+  available, so a restricted host fails closed with exit 126 instead of a
+  later, confusing readiness/test failure; an executable that cannot be
+  resolved on `PATH` is now a hard `isolated_command` failure rather than a
+  silent fallthrough that ran unwrapped and unvalidated; and the shared
+  workspace copy now rejects (fails the whole copy closed) any symlink whose
+  resolved target lands outside the copied tree, since `copytree(...,
+  symlinks=True)` otherwise preserves an escaping symlink as a live link
+  inside the bind-mounted `/workspace`.
+- (Devin review 반영, 후속 라운드) 같은 sandboxed web E2E isolation 헬퍼에 두 건을 추가로
+  hardening했습니다: (1) `_probe_isolation_capability`가 이제 `isolated_command`가 실제로
+  수행하는 모든 연산(`--new-session`, `/tmp` tmpfs, 실제 명령이 사용하는 것과 동일한 mount
+  point로의 쓰기 가능한 bind+chdir)을 진짜 임시 디렉터리로 그대로 재현합니다 — 이전의 축소된
+  probe는 이 중 하나를 거부하는 host에서는 통과했다가 실제 서비스 실행에서만 실패할 수
+  있었습니다. (2) `scripts/ci/sandboxed_verify.py`의 `copy_workspace` 기본 제외 목록에
+  자격증명 관련 dotfile/디렉터리(`.env*`, `.netrc`, `.npmrc`, `.pypirc`, `.pgpass`,
+  `.git-credentials`, `.ssh`, `.gnupg`, `.aws`, `.kube`, `.docker`)를 추가했습니다 — 쓰기
+  가능한 `/workspace` mount는 테스트 대상 명령이 읽고 쓸 수 있으므로, repo checkout에 우연히
+  존재하는 자격증명 파일이 그대로 복사되어서는 안 됩니다(로그·per-command home은 명령이 실제로
+  써야 하므로 의도적으로 동일 mount 안에 유지).
 - Fix two live-on-`main` regressions Devin Review found immediately after
   PRs #1456 and #1459 merged (both bypass-merged past the org-wide
   `opencode-review` outage; these hotfixes correct real defects the local

docs/doctoring/sandboxed-web-command-isolation.md

@@ -0,0 +1,85 @@
+# Sandboxed web command isolation
+
+`sandboxed_web_e2e.py` requires Linux `bubblewrap` (`bwrap`) by default. Each
+backend, frontend, and E2E command runs with a fresh writable `tmpfs` root and
+`/tmp`, plus one writable copied-repository bind at `/workspace`; the copied
+repository and temporary homes are mapped there. Host runtime roots and the
+minimal `/etc` identity, DNS, and time files are mounted read-only, so the host
+filesystem is not reachable through absolute paths or `..` traversal.
+
+Before wrapping a command, the helper resolves its executable and rejects paths
+outside the read-only system roots mounted by bubblewrap. A tool installed in a
+host-only location must be installed into one of those roots or the run exits
+with code `126` before any service starts; the result marker records that code
+and the selected backend. An executable that cannot be resolved on `PATH` at
+all is rejected the same way — it is never handed unvalidated to bubblewrap or
+the shell to resolve on its own.
+
+A `bwrap` binary discovered on `PATH` is not by itself proof that isolation
+works: a restricted host (unprivileged user namespaces disabled, or a
+seccomp-restricted CI runner) can have the binary present yet unable to create
+the requested namespaces. Before starting either service, `isolation_backend`
+runs a bounded, cheap capability preflight that mirrors *every* operation
+`isolated_command` actually performs — new-session creation, the new PID
+namespace, tmpfs root, the standard read-only binds, `/proc`, `/dev`, a tmpfs
+`/tmp`, and a writable bind+chdir into the same mount point real commands run
+from, exercised against a real (throwaway) temp directory rather than a
+trivial no-op. A reduced probe that skips one of these can pass on a host that
+specifically denies that operation, then fail later once a real service
+starts; mirroring the full set closes that gap. A non-zero exit, or a failure
+to even launch the probe, is classified as isolation-unavailable and exits
+with code `126`, the same as a missing `bwrap` binary, instead of surfacing
+later as a confusing readiness or test failure.
+
+Use `--isolation disabled` only for trusted local debugging. The result marker
+records the requested mode and resolved backend so CI evidence cannot be
+mistaken for an OS-isolated run. If required isolation is unavailable, the
+command exits with code `126` before starting any service.
+
+The workspace copy this helper and `sandboxed_verify.py` share
+(`sandboxed_verify.copy_workspace`) preserves symlinks rather than
+dereferencing them. Under `--isolation required`, a symlink whose absolute
+target is not one of the explicitly bound paths already dangles safely
+(`ENOENT`) inside bubblewrap's `tmpfs` root — verified empirically against
+this code path. That containment does not extend to two paths that share the
+same copy step: `--isolation disabled` (documented as trusted local debugging
+only, but the copy itself makes no such distinction) runs the wrapped commands
+directly on the host with no OS sandboxing at all, and `sandboxed_verify.py`'s
+own verification command never runs inside bubblewrap in the first place. In
+both, a repository-supplied symlink whose target is an absolute host path, or
+a relative path with enough `..` segments to exit the copy, remains a live
+symlink that a command following it can use to read or write host files
+outside the intended workspace. Every symlink under the copy is therefore
+resolved and checked against the workspace root immediately after
+`shutil.copytree`, in `copy_workspace` itself so both callers get the same
+protection; the first one found to escape fails the whole copy closed rather
+than being silently dropped or repaired.
+
+The writable `/workspace` mount is a copy of the caller's repository checkout,
+not the checkout itself. `copy_workspace` (`scripts/ci/sandboxed_verify.py`)
+excludes VCS/cache/build noise by default, and now also excludes common
+credential-bearing dotfiles/dirs a checkout can carry (`.env*`, `.netrc`,
+`.npmrc`, `.pypirc`, `.pgpass`, `.git-credentials`, `.ssh`, `.gnupg`, `.aws`,
+`.kube`, `.docker`) so a repository that happens to have one of these present
+at copy time never rides along into the sandboxed command's writable,
+readable mount. The broad `.env*` exclusion has one deliberate carve-out:
+`DEFAULT_ENV_TEMPLATE_ALLOWLIST` (`.env.example`, `.env.sample`,
+`.env.template`) still copies those committed, secret-free dotenv templates
+through, since verification commands read them for local defaults; a caller
+can still force one of those names back out with an explicit `--ignore`.
+Logs and the scrubbed per-command home directories are
+intentionally part of that same writable mount — the tested command needs to
+write them — this exclusion list narrows what "writable and readable by the
+command under test" actually contains; it does not attempt to split the mount
+by service.
+
+Readiness polling remains loopback-only and does not follow redirects. Invalid
+readiness URLs are reported as a coded readiness failure (`125`) rather than an
+uncaught traceback. The network declaration is evidence metadata; callers that
+need stronger network policy must run this helper inside a network-restricted
+runner or container.
+
+## References
+
+MITRE. (2026). *CWE-918: Server-side request forgery (SSRF)*.
+https://cwe.mitre.org/data/definitions/918.html

docs/doctoring/sandboxed-web-readiness-loopback-boundary.md

@@ -17,6 +17,16 @@ subdomains, cloud-metadata link-local addresses, missing hosts, and
 userinfo-confused URLs such as `http://user@127.0.0.1/`. A mapped public
 address such as `::ffff:8.8.8.8` cannot pass merely because it is IPv6.
 
+The port is validated too: `urllib.parse.ParseResult.port` is accessed inside
+the same function and any `ValueError` it raises (a non-numeric port such as
+`:abc`, or one out of the 0-65535 range) is re-raised as the same `ValueError`
+class every other check here raises. Before this, a malformed port passed the
+URL parse silently — the port was never read — and only surfaced later as an
+uncaught `http.client.InvalidURL` from the HTTP client itself, a class that is
+neither `ValueError` nor `urllib.error.URLError` and so was not covered by
+`main`'s exit-125 handling. It now fails the same way every other rejection
+in this function does, before any request opens.
+
 The boundary uses the standard library rather than a second address table.
 It therefore follows the runtime's maintained special-purpose definitions and
 keeps one fail-closed validation point before any network request. Do not add
@@ -40,16 +50,19 @@ The regression exercises literal `localhost`, a trailing-dot `localhost.`,
 `127.0.0.1`, another address in `127.0.0.0/8`, IPv6 `::1`, mapped loopback
 `::ffff:127.0.0.1`, an unspecified address, a `.localhost` subdomain, a
 public hostname, the common cloud metadata address, mapped public IPv6,
-userinfo, a missing host, and poisoned localhost resolution (public A,
-mapped public AAAA, empty answers, resolver errors, and non-IP answers).
-The existing no-redirect test continues to prove that an allowed readiness
-endpoint cannot redirect the poller across the boundary.
+userinfo, a missing host, poisoned localhost resolution (public A,
+mapped public AAAA, empty answers, resolver errors, and non-IP answers), and
+a non-numeric or out-of-range port on both the backend and frontend readiness
+URL, checked through both the standalone function and a `main()` run that
+never starts a service. The existing no-redirect test continues to prove that
+an allowed readiness endpoint cannot redirect the poller across the boundary.
 
 ```mermaid
 flowchart TD
   Url["Readiness URL"]
   Scheme{"http or https?"}
   Userinfo{"userinfo present?"}
+  Port{"port numeric and 0-65535?"}
   Host{"loopback IP, or localhost whose every resolved answer is loopback?"}
   Open["Poll with redirects disabled"]
   Reject["Fail closed before any request"]
@@ -58,7 +71,9 @@ flowchart TD
   Scheme -->|"no"| Reject
   Scheme -->|"yes"| Userinfo
   Userinfo -->|"yes"| Reject
-  Userinfo -->|"no"| Host
+  Userinfo -->|"no"| Port
+  Port -->|"no"| Reject
+  Port -->|"yes"| Host
   Host -->|"no"| Reject
   Host -->|"yes"| Open
 ```

docs/product-technical-gap-baseline.md

@@ -1292,6 +1292,68 @@ conflicting** PRs address pieces of this:
   currently blocked by the sidecar-preflight outage above, so neither could
   be re-reviewed to a genuine pass yet regardless of which approach wins.
 
+## 2026-08-30 PR #1347 Devin Review 6건 검증: 4건 실재 결함 수정, 2건 확인 후 해소
+
+`ContextualWisdomLab/.github#1347` (`fix/sandboxed-web-e2e-isolation-clean`,
+bubblewrap 격리 + SSRF-safe readiness-URL 검증)의 commit `7ac8298b` 기준 Devin
+Review 미해결 6건을 HEAD 코드 기준으로 개별 재검증했다. Finding 텍스트를 그대로
+신뢰하지 않고 각각 실제 동작을 재현해 확인했다.
+
+- **Finding 1 (🟡 malformed readiness port, line 423) — 실재.**
+  `require_loopback_readiness_url`는 `parsed.port`를 한 번도 읽지 않아, 비숫자
+  포트(`:abc`)는 `urllib.parse`를 그대로 통과한 뒤 `http.client.InvalidURL`을
+  발생시켰다 — 이 예외는 `ValueError`도 `urllib.error.URLError`도 아니어서
+  `main()`의 어떤 핸들러에도 잡히지 않고 스크립트가 uncaught traceback으로
+  죽는다(재현 확인). `parsed.port` 접근을 함수 안으로 추가해 동일한
+  `ValueError` 클래스로 통일했다. 백엔드/프런트엔드 readiness URL 양쪽에 대해
+  비숫자·범위초과 포트 테스트를 추가.
+- **Finding 2 (🟡 installed-but-unusable isolation, line 124) — 실재.**
+  `isolation_backend`는 `shutil.which("bwrap")`만 확인하고 실제 namespace 생성
+  가능 여부는 전혀 검증하지 않았다. `isolated_command`가 실제로 쓰는 것과 같은
+  최소 namespace/mount 구성(new PID ns, tmpfs root, 표준 read-only bind,
+  `/proc`, `/dev`, tmpfs `/tmp`)으로 현재 인터프리터의 no-op(`-c pass`)을
+  5초 timeout으로 실행하는 preflight를 추가했다. 실패 시 exit 126로 조기
+  분류.
+- **Finding 3 (📝 child-executable containment, line 163) — 정보성, 정확함.**
+  `--unshare-pid` + 암묵적 mount namespace는 wrapped 프로세스가 낳는 모든
+  자손 프로세스에도 적용되므로 추가 escape 경로가 없음을 코드로 확인. 코드
+  변경 없이 스레드에 확인 회신.
+- **Finding 4 (📝 mapped-home writability, line 135) — 정보성, 정확함.**
+  `_sandbox_environment`가 `HOME` 등을 `/workspace` 하위로 재매핑하고,
+  `sandboxed_verify.scrubbed_env`가 그 경로를 미리 생성하며, `isolated_command`가
+  동일 sandbox_root를 `--bind`(read-write)로 마운트하므로 재매핑된 홈이 실제로
+  존재하고 쓰기 가능함을 확인. 코드 변경 없이 회신.
+- **Finding 5 (🟥 workspace symlink escape, line 188) — 실재, 최우선 처리.**
+  `sandboxed_verify.copy_workspace`가 `shutil.copytree(..., symlinks=True)`를
+  써서 심볼릭 링크를 역참조 없이 그대로 보존한다는 것을 확인. 저장소에 포함된
+  심볼릭 링크가 절대경로 또는 `..` 다단 상대경로로 복사 트리 바깥을 가리키면,
+  복사 후에도 그 링크가 살아있어 `/workspace`에 bind-mount된 이후 이를
+  따라가는 명령이 sandbox 경계 밖 호스트 파일에 접근할 수 있다. 복사 직후
+  트리 전체를 순회(`rglob`, 심볼릭 디렉터리 내부로는 재귀하지 않음 — 순환
+  링크로 인한 무한 루프/과다 순회 방지)하며 모든 심볼릭 링크의 최종 resolve
+  경로가 sandbox root 하위인지 검증하고, 하나라도 벗어나면 복사 전체를
+  `ValueError`로 fail-closed 처리하도록 `_reject_escaping_symlinks`를 추가.
+  절대경로 escape, `../..` 상대경로 escape, 디렉터리 심볼릭 링크 escape,
+  풀 수 없는 순환 심볼릭 링크(RuntimeError/OSError 양쪽 Python 버전 차이
+  모두 처리) 각각에 대한 회귀 테스트와, 내부 상대 심볼릭 링크는 그대로
+  보존되는지 확인하는 회귀 테스트를 추가했다.
+- **Finding 6 (🟨 unresolved-executable bypass, line 156) — 실재.**
+  `isolated_command`는 `shutil.which(argv[0])`가 `None`을 반환하면 전체
+  검증 블록을 건너뛰고 원본 argv를 그대로 bubblewrap에 넘겼다 — 이 버그를
+  그대로 문서화하고 있던 기존 테스트
+  (`test_isolated_command_allows_unresolved_executable_for_bwrap`)를 발견,
+  fail-closed로 전환하는 테스트로 교체했다. 해석 실패 시 다른 검증과 동일한
+  `RuntimeError`(exit 126 경로)를 던지도록 수정.
+
+수정 파일: `scripts/ci/sandboxed_web_e2e.py`, `scripts/ci/sandboxed_verify.py`,
+`tests/test_sandboxed_web_e2e.py`, `tests/test_sandboxed_verify.py`,
+`docs/doctoring/sandboxed-web-command-isolation.md`,
+`docs/doctoring/sandboxed-web-readiness-loopback-boundary.md`, `CHANGELOG.md`.
+전체 스위트(`pytest tests`, 1924 passed) 및 대상 두 모듈 100% line/branch
+coverage, 100% docstring coverage(`interrogate`), `ruff check` 모두 통과 확인.
+GitHub 스레드 6건 각각에 회신하고, 실재 결함 4건 + 정보성 확인 2건 총 6건
+모두 resolve 처리.
+
 ## 2026-08-30 sidecar preflight `max_tokens`: explicit owner critique, ADR-0005 (revised after Devin Review)
 
 Direct owner feedback after #1436's `max_tokens` 16→4096 raise moved the sidecar's gateway preflight