From 117ecc009b02fa4862d999f29f514d4b4e017ed5 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Tue, 4 Aug 2026 21:42:09 +0000 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITIC?= =?UTF-8?q?AL/HIGH]=20Fix=20Information=20Disclosure=20in=20Subprocess=20E?= =?UTF-8?q?rrors?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 의도하지 않은 시크릿 유출을 방지하기 위해 `scripts/ci/sandboxed_verify.py` 및 `scripts/ci/sandboxed_web_e2e.py`의 subprocess 에러(예: TimeoutExpired) 발생 시 표준 출력 및 표준 에러를 `redact_text`를 통해 마스킹 처리했습니다. - 보안 취약점을 예방하기 위해 `scripts/ci/sandboxed_web_e2e.py` 내의 `subprocess.Popen` 및 `subprocess.run` 호출 시 `shell=False` 옵션을 명시적으로 추가했습니다. - `.jules/sentinel.md` 파일에 새로운 보안 학습 내용을 기록했습니다. --- .jules/sentinel.md | 4 ++++ scripts/ci/sandboxed_verify.py | 11 +++++++---- scripts/ci/sandboxed_web_e2e.py | 8 +++++--- tests/test_sandboxed_web_e2e.py | 4 ++-- 4 files changed, 18 insertions(+), 9 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index be2dfa4bb..e4bd66d83 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -35,3 +35,7 @@ **Vulnerability:** Command Injection **Learning:** Fixing a `shell=True` vulnerability by replacing it with `shell=False` and wrapping the command string in `["/bin/bash", "-lc", command]` is incomplete and still leaves the code vulnerable to shell injection. It acts as security theater, as it misleads linters while executing untrusted input via the bash wrapper. The vulnerability was still present in `sandboxed_web_e2e.py`. **Prevention:** Remove `/bin/bash` wrapper from `subprocess` calls in CI scripts. Always use `shlex.split(command)` to safely parse strings into a list of arguments and pass the list directly to `subprocess.Popen` or `subprocess.run`. +## 2026-08-04 - Prevent Secret Leakage in Subprocess Error Traces +**Vulnerability:** Information Disclosure / Secret Leakage +**Learning:** When a subprocess commands times out or fails (e.g., `TimeoutExpired`), simply printing the captured `stdout` and `stderr` can inadvertently expose sensitive credentials, API keys, or tokens in the CI logs. Relying on an `ImportError` fallback for the redaction tool can silently fail open and bypass redaction. +**Prevention:** Always use `scripts.ci.redact_sensitive_log.redact_text` to scrub sensitive tokens before printing subprocess outputs or timeout traces. Ensure the redaction module is imported unconditionally (e.g., by establishing the repository root on `sys.path`) to fail closed and prevent any possibility of unredacted logging. diff --git a/scripts/ci/sandboxed_verify.py b/scripts/ci/sandboxed_verify.py index aace18d45..3650893c3 100644 --- a/scripts/ci/sandboxed_verify.py +++ b/scripts/ci/sandboxed_verify.py @@ -14,6 +14,9 @@ from collections.abc import Sequence from pathlib import Path +sys.path.insert(0, str(Path(__file__).resolve().parents[2])) + +from scripts.ci.redact_sensitive_log import redact_text DEFAULT_IGNORE = ( ".git", @@ -169,8 +172,8 @@ def timeout_output_text(value: str | bytes | None) -> str: if value is None: return "" if isinstance(value, bytes): - return value.decode(errors="replace") - return value + return redact_text(value.decode(errors="replace")) + return redact_text(value) def emit_result( @@ -219,9 +222,9 @@ def main(argv: Sequence[str] | None = None) -> int: try: completed = run_command(args.command, copied_repo, env, args.timeout) if completed.stdout: - print(completed.stdout, end="") + print(redact_text(completed.stdout), end="") if completed.stderr: - print(completed.stderr, end="", file=sys.stderr) + print(redact_text(completed.stderr), end="", file=sys.stderr) exit_code = completed.returncode except subprocess.TimeoutExpired as exc: stdout = timeout_output_text(exc.stdout) diff --git a/scripts/ci/sandboxed_web_e2e.py b/scripts/ci/sandboxed_web_e2e.py index ae0c3105a..344b6609b 100644 --- a/scripts/ci/sandboxed_web_e2e.py +++ b/scripts/ci/sandboxed_web_e2e.py @@ -22,7 +22,7 @@ sys.path.insert(0, str(Path(__file__).resolve().parents[2])) from scripts.ci import sandboxed_verify - +from scripts.ci.redact_sensitive_log import redact_text RESULT_MARKER = "SANDBOXED_WEB_E2E_RESULT" @@ -110,6 +110,7 @@ def start_service(label: str, command: str, cwd: Path, env: dict[str, str], logs stdout=log_file, stderr=subprocess.STDOUT, start_new_session=True, + shell=False, ) log_file.close() return Service(label=label, command=command, process=process, log_path=log_path) @@ -146,6 +147,7 @@ def run_shell(command: str, cwd: Path, env: dict[str, str], timeout: int) -> sub stderr=subprocess.PIPE, timeout=timeout, check=False, + shell=False, ) @@ -232,9 +234,9 @@ def main(argv: Sequence[str] | None = None) -> int: try: completed = run_shell(args.e2e_cmd, copied_repo, env, args.e2e_timeout) if completed.stdout: - print(completed.stdout, end="") + print(redact_text(completed.stdout), end="") if completed.stderr: - print(completed.stderr, end="", file=sys.stderr) + print(redact_text(completed.stderr), end="", file=sys.stderr) exit_code = completed.returncode return exit_code except subprocess.TimeoutExpired as exc: diff --git a/tests/test_sandboxed_web_e2e.py b/tests/test_sandboxed_web_e2e.py index 6e092c293..d289331af 100644 --- a/tests/test_sandboxed_web_e2e.py +++ b/tests/test_sandboxed_web_e2e.py @@ -181,13 +181,13 @@ def fake_run(*args, **kwargs): assert service.command == "npm run dev" assert service.log_path == tmp_path / "backend.log" assert popen_calls[0][0] == (["npm", "run", "dev"],) - assert "shell" not in popen_calls[0][1] + assert popen_calls[0][1]["shell"] is False assert "executable" not in popen_calls[0][1] assert popen_calls[0][1]["start_new_session"] is True assert completed.returncode == 7 assert run_calls[0][0] == (["npm", "test"],) assert run_calls[0][1]["timeout"] == 5 - assert "shell" not in run_calls[0][1] + assert run_calls[0][1]["shell"] is False assert "executable" not in run_calls[0][1] From 59c2c58d59bb37463a5704130d44c23f172f9afd Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Tue, 4 Aug 2026 22:35:38 +0000 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITIC?= =?UTF-8?q?AL/HIGH]=20Fix=20Information=20Disclosure=20in=20Subprocess=20E?= =?UTF-8?q?rrors?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 의도하지 않은 시크릿 유출을 방지하기 위해 `scripts/ci/sandboxed_verify.py` 및 `scripts/ci/sandboxed_web_e2e.py`의 subprocess 에러(예: TimeoutExpired) 발생 시 표준 출력 및 표준 에러를 `redact_text`를 통해 마스킹 처리했습니다. - 보안 학습 사항을 `.jules/sentinel.md` 파일에 한국어로 기록했습니다. - 누락되었던 테스트 커버리지를 추가하여 테스트 요구 사항을 준수했습니다. --- .jules/sentinel.md | 6 +++--- scripts/ci/sandboxed_web_e2e.py | 2 -- tests/test_sandboxed_verify.py | 2 ++ tests/test_sandboxed_web_e2e.py | 14 ++++++++------ 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index e4bd66d83..f12308142 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -36,6 +36,6 @@ **Learning:** Fixing a `shell=True` vulnerability by replacing it with `shell=False` and wrapping the command string in `["/bin/bash", "-lc", command]` is incomplete and still leaves the code vulnerable to shell injection. It acts as security theater, as it misleads linters while executing untrusted input via the bash wrapper. The vulnerability was still present in `sandboxed_web_e2e.py`. **Prevention:** Remove `/bin/bash` wrapper from `subprocess` calls in CI scripts. Always use `shlex.split(command)` to safely parse strings into a list of arguments and pass the list directly to `subprocess.Popen` or `subprocess.run`. ## 2026-08-04 - Prevent Secret Leakage in Subprocess Error Traces -**Vulnerability:** Information Disclosure / Secret Leakage -**Learning:** When a subprocess commands times out or fails (e.g., `TimeoutExpired`), simply printing the captured `stdout` and `stderr` can inadvertently expose sensitive credentials, API keys, or tokens in the CI logs. Relying on an `ImportError` fallback for the redaction tool can silently fail open and bypass redaction. -**Prevention:** Always use `scripts.ci.redact_sensitive_log.redact_text` to scrub sensitive tokens before printing subprocess outputs or timeout traces. Ensure the redaction module is imported unconditionally (e.g., by establishing the repository root on `sys.path`) to fail closed and prevent any possibility of unredacted logging. +**Vulnerability:** 정보 노출 / 시크릿 유출 (Information Disclosure / Secret Leakage) +**Learning:** 서브프로세스 명령어가 시간 초과되거나 실패할 때(예: `TimeoutExpired`), 캡처된 `stdout` 및 `stderr`를 단순히 출력하게 되면 CI 로그에 민감한 자격 증명, API 키 또는 토큰이 의도치 않게 노출될 수 있습니다. redaction 도구에 대해 `ImportError` 예외 처리에 의존할 경우, 조용히 실패하여 redaction 과정을 우회할 위험이 있습니다. +**Prevention:** 서브프로세스 출력이나 시간 초과 에러 로그를 출력하기 전에는 항상 `scripts.ci.redact_sensitive_log.redact_text`를 사용하여 민감한 토큰을 스크러빙해야 합니다. redaction 모듈이 무조건적으로 임포트되도록 보장하여(예: `sys.path`에 저장소 루트를 명시적으로 추가하여), 임포트 실패 시 안전하게 시스템을 종료하고 필터링되지 않은 로그가 노출될 가능성을 원천 차단하십시오. diff --git a/scripts/ci/sandboxed_web_e2e.py b/scripts/ci/sandboxed_web_e2e.py index 344b6609b..39e2fed16 100644 --- a/scripts/ci/sandboxed_web_e2e.py +++ b/scripts/ci/sandboxed_web_e2e.py @@ -110,7 +110,6 @@ def start_service(label: str, command: str, cwd: Path, env: dict[str, str], logs stdout=log_file, stderr=subprocess.STDOUT, start_new_session=True, - shell=False, ) log_file.close() return Service(label=label, command=command, process=process, log_path=log_path) @@ -147,7 +146,6 @@ def run_shell(command: str, cwd: Path, env: dict[str, str], timeout: int) -> sub stderr=subprocess.PIPE, timeout=timeout, check=False, - shell=False, ) diff --git a/tests/test_sandboxed_verify.py b/tests/test_sandboxed_verify.py index c711f3489..b3a473417 100644 --- a/tests/test_sandboxed_verify.py +++ b/tests/test_sandboxed_verify.py @@ -85,6 +85,8 @@ def test_timeout_output_text_normalizes_subprocess_payloads(): assert sandboxed_verify.timeout_output_text(None) == "" assert sandboxed_verify.timeout_output_text(b"byte-output") == "byte-output" assert sandboxed_verify.timeout_output_text("text-output") == "text-output" + assert sandboxed_verify.timeout_output_text("my ghp_123456789012345678901234567890123456 token") == "my [REDACTED] token" + assert sandboxed_verify.timeout_output_text(b"my ghp_123456789012345678901234567890123456 token") == "my [REDACTED] token" def test_main_runs_command_in_copy_without_mutating_source(tmp_path, capsys): diff --git a/tests/test_sandboxed_web_e2e.py b/tests/test_sandboxed_web_e2e.py index d289331af..2d947d280 100644 --- a/tests/test_sandboxed_web_e2e.py +++ b/tests/test_sandboxed_web_e2e.py @@ -181,13 +181,13 @@ def fake_run(*args, **kwargs): assert service.command == "npm run dev" assert service.log_path == tmp_path / "backend.log" assert popen_calls[0][0] == (["npm", "run", "dev"],) - assert popen_calls[0][1]["shell"] is False + assert "shell" not in popen_calls[0][1] assert "executable" not in popen_calls[0][1] assert popen_calls[0][1]["start_new_session"] is True assert completed.returncode == 7 assert run_calls[0][0] == (["npm", "test"],) assert run_calls[0][1]["timeout"] == 5 - assert run_calls[0][1]["shell"] is False + assert "shell" not in run_calls[0][1] assert "executable" not in run_calls[0][1] @@ -399,7 +399,7 @@ def fake_start(label, command, cwd, env, logs_dir): return sandboxed_web_e2e.Service(label, command, DoneProcess(), log_path) def fake_run_shell(command, cwd, env, timeout): - raise subprocess.TimeoutExpired(command, timeout, output=b"e2e-out", stderr=b"e2e-err") + raise subprocess.TimeoutExpired(command, timeout, output=b"e2e-out ghp_123456789012345678901234567890123456", stderr=b"e2e-err") monkeypatch.setattr(sandboxed_web_e2e, "start_service", fake_start) monkeypatch.setattr(sandboxed_web_e2e, "wait_for_url", lambda url, timeout, service: True) @@ -423,7 +423,8 @@ def fake_run_shell(command, cwd, env, timeout): captured = capsys.readouterr() assert exit_code == 124 - assert "e2e-out" in captured.out + assert "e2e-out [REDACTED]" in captured.out + assert "ghp_123456789012345678901234567890123456" not in captured.out assert "e2e-err" in captured.err assert "e2e command timed out after 3s" in captured.err @@ -470,7 +471,7 @@ def test_sandboxed_web_e2e_reports_e2e_timeout(monkeypatch, tmp_path, capsys): repo.mkdir() def fake_run_shell(command, cwd, env, timeout): - raise subprocess.TimeoutExpired(command, timeout, output="e2e-out", stderr="e2e-err") + raise subprocess.TimeoutExpired(command, timeout, output="e2e-out ghp_123456789012345678901234567890123456", stderr="e2e-err") monkeypatch.setattr(sandboxed_web_e2e, "run_shell", fake_run_shell) @@ -491,7 +492,8 @@ def fake_run_shell(command, cwd, env, timeout): captured = capsys.readouterr() assert exit_code == 124 - assert "e2e-out" in captured.out + assert "e2e-out [REDACTED]" in captured.out + assert "ghp_123456789012345678901234567890123456" not in captured.out assert "e2e-err" in captured.err assert "e2e command timed out after 1s" in captured.err assert "SANDBOXED_WEB_E2E_RESULT" in captured.out