diff --git a/CHANGELOG.md b/CHANGELOG.md index 06b3dba425..53c23edfec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -91,6 +91,19 @@ - Remove repository-wide Actions-run inventory and cancellation from the daily organization PR recovery sweep. Native per-PR concurrency and the local exact-head coalescer remain the cancellation owners; the sweep now spends its API budget only on missed review, merge, and branch-update recovery. - Retire the standalone OSV and Scorecard pull-request workflows after both scanners moved into the required `security-scan.yml`. The organization ruleset now has seven required workflow paths, and `.github` branch protection no longer requires the duplicate `osv-scan / osv-scan` context. +### Security: structured-log secret redaction gaps + +- Fixed `scripts/ci/redact_sensitive_log.py`'s recursive JSON redaction (`_redact_json`) + only ever inspecting dict *keys* against `SENSITIVE_KEY_RE`; a provider-token-shaped + secret (`ghp_...`, `sk-...`, a Bearer header, a JWT) sitting in a string *value* under + an innocuous key (e.g. `"message"`) survived redaction unchanged when the log line was + valid JSON. `_redact_json` now also runs the same bearer/JWT/provider-token scrubbing + already used for unstructured text against every string value, factored into a shared + `_redact_token_patterns` helper. +- Narrowed the `storage[_-]?key` sensitive-key pattern so it only matches when `key` ends + the field name (e.g. `AZURE_STORAGE_KEY`), not merely contains it — `storage_key_count` + and similar diagnostic-metric field names are no longer over-redacted. + - Add `.github/actions/orchestrator-free-sidecar`, an immutable composite-action boundary that checks out the exact central control-plane revision selected by `github.action_ref` and provisions the contextual-orchestrator `orchestrator/free` gateway. Provider bootstrap remains inside the central sidecar; callers receive only the gateway URL/token-file contract for the subsequent Agent step. - Repointed 10 `scripts/ci/test_strix_quick_gate.sh` self-test assertions that had gone stale after the `pr_review_merge_scheduler.py`/`pr_review_merge_scheduler_core.py` facade/core split (#1803): they checked the now-98-line facade file for content (the exact-head branch-update guard, the squash-fallback retry, the subprocess-safety flags, the same-head Strix/OpenCode dispatch markers, and the `pr_head_ref` repository-dispatch payload) that lives in the core module instead, so they had been silently failing on every run since the split. The same repair aligns the wake-workflow list and daily recovery assertions with the current event-driven scheduler contract. A coverage/docstring version of the same gap was already fixed via #1810; this bash contract script was missed. - **Fix the `coalesce` required check crashing instead of exiting cleanly for a superseded queued run.** `current-head-run-coalescer.yml`'s own design comment documents that `current_head_run_coalescer.py` raising `CoalescingRefused` (its remembered head no longer matching the PR's live head) is "a safe no-op" — but `main()` only ever called `coalesce()` directly, so the exception raised by `coalesce()`'s own top-level live-PR-state check propagated uncaught and crashed the job with exit code 1, instead of the intended graceful no-op. Reproduced live on `ContextualWisdomLab/.github#1503` (run `33766056421`, job `100684095620`): a stale queued run drained from the org-wide Actions capacity backlog against an already-superseded head failed the required `coalesce` check with `CoalescingRefused: pull request head moved before duplicate classification`. `main()` now catches `CoalescingRefused` specifically and exits 0 with an informational message; any other exception (malformed identity, an unavailable GitHub API) still fails closed. @@ -1269,6 +1282,10 @@ Semantic Versioning where the repository publishes a release. ### Fixed +- Redact Stripe secret-key prefixes in unstructured CI evidence while + preserving unlabeled 40-character commit SHAs and other fixed-length + evidence; generic AWS and Azure values remain protected by the existing + sensitive-assignment parser instead of an overbroad length-only pattern. - Prefer the job-scoped `github.token` when the central OpenCode dispatch publishes a commit status back to the same `.github` repository. The job's declared `statuses: write` permission now reaches the endpoint instead of an diff --git a/docs/doctoring/ci-log-evidence-redaction.md b/docs/doctoring/ci-log-evidence-redaction.md new file mode 100644 index 0000000000..6f7257ed6f --- /dev/null +++ b/docs/doctoring/ci-log-evidence-redaction.md @@ -0,0 +1,51 @@ +# CI log evidence redaction + +## Incident and boundary + +PR #1242 briefly classified every standalone 40- or 88-character base64-like +value as a credential. A 40-character lowercase hexadecimal Git commit identity +therefore became `[REDACTED]`, destroying the exact-head evidence that protected +review and merge gates need. Length alone cannot distinguish an opaque secret +from a commit SHA or other legitimate evidence. + +The redactor now uses the smallest reliable boundary: + +- provider-specific, documented prefixes such as Stripe `sk_test_` and + `sk_live_` may be recognized in unstructured text; +- opaque AWS and Azure values are redacted only when a sensitive assignment or + JSON key supplies context, including `AWS_SECRET_ACCESS_KEY` and + `AZURE_STORAGE_KEY`; +- unlabeled fixed-length strings remain visible so exact commit and artifact + identities stay auditable. + +This follows OWASP's requirement to keep secrets out of logs while retaining +the security events and audit fidelity needed for investigation. It also uses +the vendors' documented key names or prefixes instead of an inferred value +shape. + +## Verification contract + +`tests/test_opencode_security_boundaries.py` uses synthetic values to prove all +four outcomes: Stripe secret prefixes are removed, labeled AWS and Azure values +are removed, and unlabeled 40- and 88-character evidence is preserved. No real +credential or provider account data is stored in the repository. + +## References + +Amazon Web Services. (n.d.). *Configuring environment variables for the AWS +CLI*. Retrieved August 23, 2026, from +https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-envvars.html + +Microsoft. (n.d.). *Authorize access to blob data with Azure CLI*. Retrieved +August 23, 2026, from +https://learn.microsoft.com/en-us/azure/storage/blobs/authorize-data-operations-cli + +OWASP Foundation. (n.d.). *Logging cheat sheet*. Retrieved August 23, 2026, +from https://cheatsheetseries.owasp.org/cheatsheets/Logging_Cheat_Sheet.html + +OWASP Foundation. (n.d.). *Secrets management cheat sheet*. Retrieved August +23, 2026, from +https://cheatsheetseries.owasp.org/cheatsheets/Secrets_Management_Cheat_Sheet.html + +Stripe. (n.d.). *API keys*. Retrieved August 23, 2026, from +https://docs.stripe.com/keys diff --git a/scripts/ci/redact_sensitive_log.py b/scripts/ci/redact_sensitive_log.py index c3a59bf6f1..fa688802fa 100644 --- a/scripts/ci/redact_sensitive_log.py +++ b/scripts/ci/redact_sensitive_log.py @@ -12,7 +12,8 @@ KEY_CHARS = frozenset("ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789_.-") SENSITIVE_KEY_RE = re.compile( r"(?:token|secret|password|passwd|credential|authorization|jwt|" - r"api[_-]?key|private[_-]?key|access[_-]?key|session[_-]?key)", + r"api[_-]?key|private[_-]?key|access[_-]?key|session[_-]?key|" + r"storage[_-]?key(?![A-Za-z0-9_-]))", re.IGNORECASE, ) JWT_RE = re.compile( @@ -24,16 +25,25 @@ r"[^\s\"'\\]+", re.IGNORECASE, ) -PROVIDER_TOKEN_RES = ( - re.compile(r"\b(?:gh[pousr]_[A-Za-z0-9_]{20,}|github_pat_[A-Za-z0-9_]{20,})\b"), - re.compile(r"\bsk-[A-Za-z0-9_-]{20,}\b"), - re.compile(r"\bxox[baprs]-[A-Za-z0-9-]{20,}\b"), - re.compile(r"\bAKIA[0-9A-Z]{16}\b"), +PROVIDER_TOKEN_RE = re.compile( + r"\b(?:gh[pousr]_[A-Za-z0-9_]{20,}|github_pat_[A-Za-z0-9_]{20,}|" + r"sk-[A-Za-z0-9_-]{20,}|" + r"xox[baprs]-[A-Za-z0-9-]{20,}|" + r"AKIA[0-9A-Z]{16}|" + r"sk_(?:test|live)_[A-Za-z0-9]{24,})\b" ) +def _redact_token_patterns(text: str) -> str: + """Redact bearer/basic headers, JWTs, and provider-token shapes in place.""" + cleaned = BEARER_RE.sub(lambda match: f"{match.group('prefix')}{REDACTED}", text) + cleaned = JWT_RE.sub(REDACTED, cleaned) + cleaned = PROVIDER_TOKEN_RE.sub(REDACTED, cleaned) + return cleaned + + def _redact_json(value: Any) -> Any: - """Recursively replace values whose JSON keys identify credentials.""" + """Recursively replace credentials identified by JSON keys or value shape.""" if isinstance(value, dict): return { key: REDACTED if SENSITIVE_KEY_RE.search(str(key)) else _redact_json(item) @@ -41,6 +51,8 @@ def _redact_json(value: Any) -> Any: } if isinstance(value, list): return [_redact_json(item) for item in value] + if isinstance(value, str): + return _redact_token_patterns(value) return value @@ -138,12 +150,7 @@ def _redact_assignments(text: str) -> str: def _redact_unstructured(text: str) -> str: """Redact credential-shaped values from non-JSON diagnostic text.""" - cleaned = _redact_assignments(text) - cleaned = BEARER_RE.sub(lambda match: f"{match.group('prefix')}{REDACTED}", cleaned) - cleaned = JWT_RE.sub(REDACTED, cleaned) - for pattern in PROVIDER_TOKEN_RES: - cleaned = pattern.sub(REDACTED, cleaned) - return cleaned + return _redact_token_patterns(_redact_assignments(text)) _JSON_VALUE_START_CHARS = frozenset('{["-0123456789tfnNI') diff --git a/tests/test_opencode_security_boundaries.py b/tests/test_opencode_security_boundaries.py index 2f968654bb..ef82ca3616 100644 --- a/tests/test_opencode_security_boundaries.py +++ b/tests/test_opencode_security_boundaries.py @@ -123,6 +123,7 @@ def test_sensitive_log_redaction_scrubs_provider_token_shapes() -> None: "openai sk-" + ("C" * 24), "slack xoxb-" + ("D" * 24), "aws AKIA" + ("E" * 16), + "stripe sk_test_" + ("F" * 24), ] ) cleaned = redactor.redact_text(source) @@ -132,7 +133,53 @@ def test_sensitive_log_redaction_scrubs_provider_token_shapes() -> None: assert "sk-" not in cleaned assert "xoxb-" not in cleaned assert "AKIA" not in cleaned - assert cleaned.count(redactor.REDACTED) == 5 + assert "sk_test_" not in cleaned + assert cleaned.count(redactor.REDACTED) == 6 + + +def test_sensitive_log_redaction_scrubs_provider_tokens_inside_structured_values() -> None: + """Provider-token-shaped values are scrubbed even under an innocuous JSON key.""" + source = json.dumps( + { + "message": "leaked classic ghp_" + ("A" * 24) + " during the run", + "note": "no secret here", + } + ) + cleaned = redactor.redact_text(source) + parsed = json.loads(cleaned) + + assert "ghp_" not in cleaned + assert parsed["message"] == f"leaked classic {redactor.REDACTED} during the run" + assert parsed["note"] == "no secret here" + + +def test_sensitive_log_redaction_storage_key_requires_an_exact_field_name() -> None: + """`storage_key`-shaped fields are redacted; unrelated metrics keep their values.""" + cleaned = redactor.redact_text( + json.dumps({"AZURE_STORAGE_KEY": "fixture-storage-secret", "storage_key_count": 3}) + ) + parsed = json.loads(cleaned) + + assert parsed["AZURE_STORAGE_KEY"] == redactor.REDACTED + assert parsed["storage_key_count"] == 3 + + assert redactor.redact_text("storage_key_count=3") == "storage_key_count=3" + assert redactor.redact_text("storage_key=hunter2") == f"storage_key={redactor.REDACTED}" + + +def test_sensitive_log_redaction_requires_context_for_fixed_length_secrets() -> None: + """Generic fixed-length values need a secret label so evidence stays usable.""" + commit_sha = "a" * 40 + opaque_evidence = "B" * 88 + + cleaned = redactor.redact_text( + f"head={commit_sha}\nevidence {opaque_evidence}\n" + f"AWS_SECRET_ACCESS_KEY={commit_sha}\nAZURE_STORAGE_KEY={opaque_evidence}\n" + ) + + assert commit_sha in cleaned + assert opaque_evidence in cleaned + assert cleaned.count(redactor.REDACTED) == 2 def test_sensitive_log_redaction_handles_lists_empty_input_and_cli(monkeypatch: pytest.MonkeyPatch) -> None: