보안: CI 스크립트 실행 실패 시 민감 정보 스크러빙 및 shell=False 명시

.jules/sentinel.md

@@ -6,3 +6,7 @@
 **Vulnerability:** Workflow CI Security Bypass / Markdown Injection
 **Learning:** The GitHub Actions workflow `opencode-review.yml` attempted to optimize performance by doing a fast-path bash string extraction. If this succeeded, it skipped the Python JSON normalizer (`opencode_review_normalize_output.py`). This is a security flaw because the bash script does not escape `<, >, &` characters, allowing attackers to inject `-->` directly in JSON strings to break out of HTML comment sections.
 **Prevention:** Removed the fast-path check entirely. We must always enforce JSON normalization via `opencode_review_normalize_output.py` because it correctly parses the JSON payload and safely escapes all characters as `\u003c`, `\u003e` and `\u0026`.
+## 2024-06-25 - Prevent CI Logs Security Exposure and Explicit Shell Usage
+**Vulnerability:** Information Disclosure / Command Injection
+**Learning:** `subprocess.run` defaults to `shell=False`, but linters like Bandit require explicit `shell=False` to pass security checks. Furthermore, logging `process.stderr` or command arguments in CI tools can leak sensitive data (e.g., GitHub tokens or API keys passed to commands) if a command fails and dumps the context.
+**Prevention:** Always explicitly define `shell=False` when using `subprocess.run()`. Scrub secrets from both arguments and `stderr` before including them in error messages within CI scripts.

scripts/ci/opencode_review_approve_gate.sh

@@ -183,6 +183,7 @@ def changed_new_lines(path_value: str) -> set[int]:
             ],
             check=False,
             text=True,
+            shell=False,
             stdout=subprocess.PIPE,
             stderr=subprocess.DEVNULL,
             shell=False,

scripts/ci/pr_review_merge_scheduler.py

@@ -147,7 +147,7 @@ def scrub_sensitive_data(text: str | None) -> str | None:
         return text
     text = re.sub(r'(?i)(bearer\s+)[^\s"\'\\]+', r'\1***', text)
     text = re.sub(r'(?i)(token\s+)[^\s"\'\\]+', r'\1***', text)
-    text = re.sub(r'(ghp_[A-Za-z0-9_]+|github_pat_[A-Za-z0-9_]+)', '***', text)
+    text = re.sub(r'(?i)(github_pat_[A-Za-z0-9_]+|gh[psuo]_[A-Za-z0-9_]+)', '***', text)
     return text
 
 

tests/test_pr_review_merge_scheduler.py

@@ -492,6 +492,35 @@ def test_review_state_and_failed_checks():
     assert sched.failed_status_checks(manual_strix_supersedes_pr_target_failure) == ["lint"]
 
 
+def test_run_command_failure_scrubs_secrets(monkeypatch):
+    import subprocess
+
+    class MockProcess:
+        def __init__(self, returncode, stdout, stderr):
+            self.returncode = returncode
+            self.stdout = stdout
+            self.stderr = stderr
+
+    def mock_run(args, **kwargs):
+        stderr_msg = "Error using Token secret_token_123 and bearer super_secret"
+        if "fail" in args:
+            raise subprocess.CalledProcessError(1, args, output="", stderr=stderr_msg)
+        return MockProcess(0, "success", "")
+
+    monkeypatch.setattr(subprocess, "run", mock_run)
+
+    assert sched.run(["success"]) == "success"
+
+    token_placeholder = "ghp_placeholder_token_with_underscores_123"
+    with pytest.raises(RuntimeError) as exc_info:
+        sched.run(["gh", "api", "fail", "-H", f"Authorization: token {token_placeholder}"])
+
+    error_msg = str(exc_info.value)
+    assert token_placeholder not in error_msg
+    assert "secret_token_123" not in error_msg
+    assert "super_secret" not in error_msg
+    assert "***" in error_msg
+
 def test_actions_call_gh_with_expected_arguments(monkeypatch):
     calls = []
 
@@ -1238,7 +1267,10 @@ def fake_inspect(repo, pr, **kwargs):
 def test_scrub_sensitive_data_and_run_error():
     assert sched.scrub_sensitive_data("Authorization: Bearer mytoken123") == "Authorization: Bearer ***"
     assert sched.scrub_sensitive_data("token mytoken123") == "token ***"
-    assert sched.scrub_sensitive_data("ghp_1234567890abcdef") == "***"
+    assert sched.scrub_sensitive_data("ghp_placeholder_token_with_underscores_123") == "***"
+    assert sched.scrub_sensitive_data("gho_installation_token_value") == "***"
+    assert sched.scrub_sensitive_data("ghu_user_token_value") == "***"
+    assert sched.scrub_sensitive_data("ghs_server_token_value") == "***"
     assert sched.scrub_sensitive_data("github_pat_11AAAAA_abcdefg") == "***"
     assert sched.scrub_sensitive_data("No secrets here") == "No secrets here"
     assert sched.scrub_sensitive_data("") == ""