From 5150cd79be921550e838b08bbec560052379e797 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 9 Oct 2026 14:32:35 +0900 Subject: [PATCH] fix(reporting): redact imported secret evidence Normalize untrusted finding envelopes through one report-safe boundary. Redact nested fields and keys, authorization/credential/provider forms, multiline and escaped delimiters, and source-aligned truncated snippets. Preserve IssueOps compatibility and add downstream regression coverage for all reports, SARIF, and control-plane persistence. --- .../report-secret-ingestion-redaction.md | 24 ++ appguardrail_core/findings.py | 110 +++++- appguardrail_core/issueops.py | 22 +- appguardrail_core/redaction.py | 183 ++++++++++ docs/TEST_STRATEGY.md | 4 +- tests/test_controlplane.py | 23 ++ tests/test_findings_core.py | 335 +++++++++++++++++- tests/test_issueops_core.py | 130 +++++++ tests/test_reports_core.py | 67 ++++ tests/test_sarif.py | 20 ++ 10 files changed, 890 insertions(+), 28 deletions(-) create mode 100644 CHANGELOG.d/report-secret-ingestion-redaction.md create mode 100644 appguardrail_core/redaction.py diff --git a/CHANGELOG.d/report-secret-ingestion-redaction.md b/CHANGELOG.d/report-secret-ingestion-redaction.md new file mode 100644 index 000000000..4dd683fe0 --- /dev/null +++ b/CHANGELOG.d/report-secret-ingestion-redaction.md @@ -0,0 +1,24 @@ +### Security + +- Normalize imported findings through one fail-closed serialization boundary: + suppress complete snippets for secret-category and genuinely sensitive-rule + findings, recursively redact raw/credential-named extension fields, and + selectively redact obvious authorization, credential-assignment, + provider-token, and JWT material from all other text before reports, SARIF, + or control-plane persistence. +- Sanitize secret-bearing mapping keys, post-coercion text, plural/camel-case + sensitive fields, arbitrary Authorization schemes, and AWS access-key + identifiers while bounding snippet inspection before pattern matching. +- Consume multiline quoted credentials, cover scoped `sk-proj-*` and + `sk-svcacct-*` tokens, validate report line numbers without deferred string + coercion, and inspect bounded lookahead so truncation cannot expose a token + prefix at the report boundary. +- Cover suffixed compound credential fields and triple-quoted/backtick values; + lookahead now marks source-aligned secret spans without moving text from + beyond the configured snippet boundary into serialized output. +- Treat escaped characters inside triple-quoted credentials as content rather + than a closing delimiter, preventing the remaining secret body from leaking. +- Preserve non-sensitive evidence and existing truncation behavior. Unknown + secret formats outside raw/credential-named fields depend on a + format-specific redaction pattern; regression fixtures use only synthetic + values. Provider-related non-secret rules do not suppress benign evidence. diff --git a/appguardrail_core/findings.py b/appguardrail_core/findings.py index 5e0c28b43..3d32d8819 100644 --- a/appguardrail_core/findings.py +++ b/appguardrail_core/findings.py @@ -4,12 +4,43 @@ from typing import Any, Iterable +from appguardrail_core.redaction import ( + redact_sensitive_prefix, + redact_sensitive_text, + redact_sensitive_value, +) + SEVERITIES = ("CRITICAL", "HIGH", "WARNING", "INFO") DEPLOY_BLOCKING_SEVERITIES = {"CRITICAL", "HIGH"} NON_BLOCKING_CONTEXTS = {"doc", "test", "example", "scanner-fixture"} _SEVERITY_ORDER = {severity: index for index, severity in enumerate(SEVERITIES)} _SEV_SET = frozenset(SEVERITIES) +_SENSITIVE_RULE_SEGMENTS = frozenset({"hardcoded"}) +_SENSITIVE_ASSET_SEGMENTS = frozenset( + { + "credential", + "credentials", + "jwt", + "password", + "secret", + "secrets", + "token", + } +) +_SENSITIVE_DISCLOSURE_SEGMENTS = frozenset( + {"checked", "committed", "disclosure", "exposure", "leak", "leaked"} +) +_SENSITIVE_RULE_IDS = frozenset( + { + "env-file-with-secrets-committed", + "next-public-secret", + "nextjs-env-secret-client-prefix", + "stripe-secret-key-client-exposure", + } +) +_REDACTED_SENSITIVE_SNIPPET = "[REDACTED: sensitive match suppressed]" +_REDACTION_LOOKAHEAD = 128 def normalize_finding( @@ -18,7 +49,9 @@ def normalize_finding( snippet_max_len: int = 400, ) -> dict[str, Any]: """Return a normalized, report-safe AppGuardrail finding dictionary.""" - normalized = dict(finding) + untrusted_finding = dict(finding) + untrusted_snippet = untrusted_finding.pop("snippet", "") + normalized = redact_sensitive_value(untrusted_finding) sev = normalized.get("severity") if type(sev) is not str or sev not in _SEV_SET: @@ -48,7 +81,17 @@ def normalize_finding( except Exception: normalized["file"] = "n/a" - if not normalized.get("line"): + line = normalized.get("line") + if type(line) is int and line > 0: + normalized["line"] = line + elif ( + type(line) is str + and len(line) <= 20 + and line.isascii() + and line.isdecimal() + ): + normalized["line"] = max(1, int(line)) + else: normalized["line"] = 1 cat = normalized.get("category") @@ -89,13 +132,20 @@ def normalize_finding( except Exception: normalized["verification"] = "Rerun AppGuardrail after remediation." - snip = normalized.get("snippet") + snip = untrusted_snippet if type(snip) is not str or not snip: try: snip = str(snip or "") except Exception: snip = "" - normalized["snippet"] = safe_report_snippet(snip, max_len=snippet_max_len) + safe_snippet = safe_report_snippet( + snip, + max_len=snippet_max_len, + rule_id=normalized["rule_id"], + category=normalized["category"], + ) + normalized = redact_sensitive_value(normalized) + normalized["snippet"] = safe_snippet return normalized @@ -194,8 +244,14 @@ def finding_sort_key(finding: dict[str, Any]) -> tuple[int, str, str]: ) -def safe_report_snippet(snippet: str, max_len: int = 400) -> str: - """Trim report evidence without carrying oversized raw snippets.""" +def safe_report_snippet( + snippet: str, + max_len: int = 400, + *, + rule_id: str = "", + category: str = "", +) -> str: + """Redact sensitive report evidence and bound the remaining snippet.""" if type(snippet) is not str: try: snippet = str(snippet or "") @@ -203,10 +259,44 @@ def safe_report_snippet(snippet: str, max_len: int = 400) -> str: snippet = "" if not snippet: return "" - snippet = snippet.replace("\r\n", "\n").replace("\r", "\n").strip() - if len(snippet) <= max_len: - return snippet - return snippet[:max_len].rstrip() + "\n...[truncated]" + normalized_rule_id = rule_id.strip().lower() if type(rule_id) is str else "" + normalized_category = category.strip().lower() if type(category) is str else "" + rule_segments = frozenset( + filter(None, normalized_rule_id.replace("_", "-").split("-")) + ) + sensitive_asset = bool(rule_segments.intersection(_SENSITIVE_ASSET_SEGMENTS)) + sensitive_asset = sensitive_asset or {"private", "key"}.issubset(rule_segments) + sensitive_asset = sensitive_asset or {"api", "key"}.issubset(rule_segments) + disclosure_semantics = bool( + rule_segments.intersection(_SENSITIVE_DISCLOSURE_SEGMENTS) + ) + if ( + normalized_category == "secrets" + or normalized_rule_id in _SENSITIVE_RULE_IDS + or bool(rule_segments.intersection(_SENSITIVE_RULE_SEGMENTS)) + or (sensitive_asset and disclosure_semantics) + ): + return _REDACTED_SENSITIVE_SNIPPET + max_len = max(0, max_len) + was_truncated = len(snippet) > max_len + inspection_len = max_len + _REDACTION_LOOKAHEAD + visible_snippet = snippet[:max_len] + inspection_snippet = ( + snippet[:inspection_len] if len(snippet) > inspection_len else snippet + ) + visible_snippet = visible_snippet.replace("\r\n", "\n").replace("\r", "\n") + inspection_snippet = inspection_snippet.replace("\r\n", "\n").replace( + "\r", "\n" + ) + if was_truncated: + snippet = redact_sensitive_prefix( + inspection_snippet, + visible_length=len(visible_snippet), + ) + else: + snippet = redact_sensitive_text(visible_snippet) + snippet = snippet.strip() + return snippet.rstrip() + ("\n...[truncated]" if was_truncated else "") def _as_tuple(value: Any) -> tuple[str, ...]: diff --git a/appguardrail_core/issueops.py b/appguardrail_core/issueops.py index 2c5695a10..e8b8c9e6a 100644 --- a/appguardrail_core/issueops.py +++ b/appguardrail_core/issueops.py @@ -6,6 +6,8 @@ import re from typing import Any +from appguardrail_core.redaction import SENSITIVE_TEXT_PATTERNS + FAILURES = {"failure", "cancelled", "timed_out", "action_required"} SECURITY_TERMS = ( "strix", @@ -21,6 +23,9 @@ DEFAULT_MAX_LOG_LINES = 200 MAX_GITHUB_RUN_ID_DIGITS = 20 +# Compatibility alias for callers that inspected the former module-level list. +SECRET_RE = list(SENSITIVE_TEXT_PATTERNS) + ANSI_RE = re.compile(r"\x1b\[[0-?]*[ -/]*[@-~]") TS_RE = re.compile( r"^\ufeff?\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d+Z[^\S\r\n]*", @@ -36,19 +41,6 @@ for separator in ("\r", "\v", "\f", "\x1c", "\x1d", "\x1e", "\x85", "\u2028", "\u2029") } ) -SECRET_RE = [ - re.compile(r"(?i)(authorization:\s*(?:bearer|token)\s+)[^\s]+"), - re.compile( - r"(?im)\b((?:api[_-]?key|token|secret|password|private[_-]?key)\s*[:=]\s*)" - r"(?:'(?:\\.|[^'\\\r\n])*(?:'|(?=\r?$))|" - r'"(?:\\.|[^"\\\r\n])*(?:"|(?=\r?$))|' - r"[^'\"\s]+['\"]?)" - ), - re.compile( - r"\b(?:gh[opsu]_[A-Za-z0-9_]{20,}|github_pat_[A-Za-z0-9_]+|sk-[A-Za-z0-9]{20,})\b" - ), - re.compile(r"\beyJ[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+\b"), -] PRIMARY_LOG_RE = [ re.compile(pattern, re.IGNORECASE) for pattern in ( @@ -112,8 +104,8 @@ def redact(log: str) -> str: if text.endswith("\n"): text = text[:-1] text = TS_RE.sub("", text) - for regex in SECRET_RE: - text = regex.sub( + for pattern in SECRET_RE: + text = pattern.sub( lambda match: ( f"{match.group(1)}[REDACTED]" if match.lastindex else "[REDACTED]" ), diff --git a/appguardrail_core/redaction.py b/appguardrail_core/redaction.py new file mode 100644 index 000000000..77d442947 --- /dev/null +++ b/appguardrail_core/redaction.py @@ -0,0 +1,183 @@ +"""Shared redaction primitives for untrusted security evidence.""" + +from __future__ import annotations + +import re +from collections.abc import Mapping +from typing import Any + +SENSITIVE_TEXT_PATTERNS = ( + re.compile( + r"(?im)(['\"]?authorization['\"]?\s*[:=]\s*)" + r"(?:'''(?:\\[\s\S]|(?!''')[\s\S])*(?:'''|\Z)|" + r'"""(?:\\[\s\S]|(?!""")[\s\S])*(?:"""|\Z)|' + r"`(?:\\[\s\S]|[^`\\])*(?:`|\Z)|" + r"'(?:\\[\s\S]|[^'\\])*(?:'|\Z)|" + r'"(?:\\[\s\S]|[^"\\])*(?:"|\Z)|' + r"[^\r\n]+)" + ), + re.compile( + r"(?im)\b(((?:[a-z][a-z0-9]*[_-])*(?:api[_-]?key|access[_-]?key|" + r"access[_-]?key[_-]?id|secret[_-]?access[_-]?key(?:[_-]?id)?|" + r"client[_-]?secret|access[_-]?token|" + r"refresh[_-]?token|token|secret|password|private[_-]?key|credential)" + r")\s*[:=]\s*)" + r"(?:'''(?:\\[\s\S]|(?!''')[\s\S])*(?:'''|\Z)|" + r'"""(?:\\[\s\S]|(?!""")[\s\S])*(?:"""|\Z)|' + r"`(?:\\[\s\S]|[^`\\])*(?:`|\Z)|" + r"'(?:\\[\s\S]|[^'\\])*(?:'|\Z)|" + r'"(?:\\[\s\S]|[^"\\])*(?:"|\Z)|' + r"[^'\"\s]+['\"]?)" + ), + re.compile( + r"\b(?:" + r"gh[opsu]_[A-Za-z0-9_]{20,}|github_pat_[A-Za-z0-9_]+|" + r"sk-[A-Za-z0-9]{20,}|sk-(?:ant|proj|svcacct)-[A-Za-z0-9_-]{20,}|" + r"sk_(?:live|test)_[A-Za-z0-9]{12,}|" + r"(?:AKIA|ASIA)[A-Z0-9]{16}|AIza[0-9A-Za-z_-]{35}|" + r"xox[baprs]-[A-Za-z0-9-]{10,}|" + r"SG\.[A-Za-z0-9_-]{16,}\.[A-Za-z0-9_-]{16,}|" + r"npm_[A-Za-z0-9]{20,}|pypi-[A-Za-z0-9_-]{20,}" + r")\b" + ), + re.compile(r"\beyJ[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+\b"), +) +_SENSITIVE_FIELD_PARTS = frozenset( + { + "api_key", + "authorization", + "credential", + "password", + "private_key", + "raw_evidence", + "raw_output", + "raw_payload", + "raw_response", + "raw", + "request_body", + "response_body", + "secret", + "source_code", + "snippet", + "findings", + "token", + } +) + + +def redact_sensitive_text(text: str) -> str: + """Replace obvious credential material while preserving surrounding evidence.""" + for pattern in SENSITIVE_TEXT_PATTERNS: + text = pattern.sub( + lambda match: ( + f"{match.group(1)}[REDACTED]" if match.lastindex else "[REDACTED]" + ), + text, + ) + return text + + +def redact_sensitive_prefix(text: str, *, visible_length: int) -> str: + """Redact a source-aligned prefix while using bounded suffix lookahead.""" + visible_length = max(0, min(visible_length, len(text))) + secret_spans: list[tuple[int, int]] = [] + for pattern in SENSITIVE_TEXT_PATTERNS: + for match in pattern.finditer(text): + secret_start = match.end(1) if match.lastindex else match.start() + secret_end = match.end() + if secret_start < visible_length and secret_end > secret_start: + secret_spans.append( + (secret_start, min(secret_end, visible_length)) + ) + if not secret_spans: + return text[:visible_length] + + merged_spans: list[tuple[int, int]] = [] + for span_start, span_end in sorted(secret_spans): + if merged_spans and span_start <= merged_spans[-1][1]: + prior_start, prior_end = merged_spans[-1] + merged_spans[-1] = (prior_start, max(prior_end, span_end)) + else: + merged_spans.append((span_start, span_end)) + + chunks: list[str] = [] + cursor = 0 + for span_start, span_end in merged_spans: + chunks.extend((text[cursor:span_start], "[REDACTED]")) + cursor = span_end + chunks.append(text[cursor:visible_length]) + return "".join(chunks) + + +def redact_sensitive_value(value: Any, *, field_name: str = "") -> Any: + """Recursively redact secret-bearing fields and text in finding extensions.""" + separated_name = re.sub(r"(?<=[a-z0-9])(?=[A-Z])", "_", field_name) + normalized_name = re.sub( + r"[^a-z0-9]+", "_", separated_name.casefold() + ).strip("_") + name_parts = ( + frozenset(normalized_name.split("_")) if normalized_name else frozenset() + ) + compound_name = normalized_name.replace("_", "") + sensitive_name = bool( + name_parts.intersection( + { + "authorization", + "credential", + "credentials", + "findings", + "password", + "passwords", + "raw", + "secret", + "secrets", + "snippet", + "snippets", + "token", + "tokens", + } + ) + or normalized_name in _SENSITIVE_FIELD_PARTS + or {"api", "key"}.issubset(name_parts) + or {"access", "key"}.issubset(name_parts) + or {"private", "key"}.issubset(name_parts) + or {"api", "keys"}.issubset(name_parts) + or {"access", "keys"}.issubset(name_parts) + or {"private", "keys"}.issubset(name_parts) + or compound_name + in { + "apikey", + "apikeys", + "credentialdata", + "privatekey", + "privatekeys", + "rawevidence", + "rawoutput", + "rawpayload", + "rawresponse", + } + ) + if sensitive_name: + return "[REDACTED]" + if isinstance(value, str): + return redact_sensitive_text(value) + if isinstance(value, Mapping): + sanitized: dict[Any, Any] = {} + for key, child in value.items(): + safe_key = redact_sensitive_text(key) if isinstance(key, str) else key + if safe_key in sanitized: + base_key = safe_key if isinstance(safe_key, str) else repr(safe_key) + suffix = 2 + while f"{base_key}#{suffix}" in sanitized: + suffix += 1 + safe_key = f"{base_key}#{suffix}" + sanitized[safe_key] = redact_sensitive_value( + child, + field_name=key if isinstance(key, str) else "", + ) + return sanitized + if isinstance(value, tuple): + return tuple(redact_sensitive_value(child) for child in value) + if isinstance(value, list): + return [redact_sensitive_value(child) for child in value] + return value diff --git a/docs/TEST_STRATEGY.md b/docs/TEST_STRATEGY.md index 2dcf63f5c..70a4fb2a6 100644 --- a/docs/TEST_STRATEGY.md +++ b/docs/TEST_STRATEGY.md @@ -73,7 +73,7 @@ Autofix tests prove preview/apply idempotence and semantics preservation for eac ## Reporting/SBOM tests -Normalize deterministic finding envelopes, SARIF validity, buyer/founder/agency/fix-pack rendering, raw-secret omission, evidence warnings, lockfile/version provenance, SBOM deterministic component identity, and organization bundle manifest integrity. +Normalize deterministic finding envelopes, SARIF validity, buyer/founder/agency/fix-pack rendering, raw-secret omission (including untrusted imported findings), evidence warnings, lockfile/version provenance, SBOM deterministic component identity, and organization bundle manifest integrity. Redaction regressions must cover secret-category suppression, sensitive-rule suppression despite incorrect category metadata, raw/credential-named extension fields and mapping keys, multiline single/double/triple/backtick assignments and escaped delimiters, obvious and scoped provider credentials, post-coercion text and line-number validation, source-aligned truncation-boundary lookahead, every report type, SARIF and control-plane persistence, and preservation of benign provider-related evidence. ## Performance @@ -85,4 +85,4 @@ Verify immutable action refs, RCA-first feasibility, no provider/reviewer secret ## Release acceptance -A release requires one exact integrated protected head satisfying detector positive/negative obligations, tenant/network security, exact coverage, packaging/SBOM/provenance, migration/rollback where state changes, independent review, and post-publish smoke. \ No newline at end of file +A release requires one exact integrated protected head satisfying detector positive/negative obligations, tenant/network security, exact coverage, packaging/SBOM/provenance, migration/rollback where state changes, independent review, and post-publish smoke. diff --git a/tests/test_controlplane.py b/tests/test_controlplane.py index 830f96930..4c5384ffd 100644 --- a/tests/test_controlplane.py +++ b/tests/test_controlplane.py @@ -54,6 +54,29 @@ def test_add_and_list_scan_counts(): assert len(full["findings"]) == 2 +def test_add_scan_does_not_persist_untrusted_secret_evidence(): + synthetic_token = "ghp_" + "d" * 24 + conn = connect(":memory:") + oid, _ = create_org(conn, "Acme") + + scan = add_scan( + conn, + oid, + [ + { + "severity": "HIGH", + "rule_id": "external-finding-42", + "message": f"Observed token={synthetic_token}", + "raw_evidence": {"body": synthetic_token}, + } + ], + ) + persisted = get_scan(conn, oid, scan["id"])["findings"][0] + + assert synthetic_token not in json.dumps(persisted) + assert persisted["raw_evidence"] == "[REDACTED]" + + def test_tenant_isolation(): conn = connect(":memory:") a, _ = create_org(conn, "Acme") diff --git a/tests/test_findings_core.py b/tests/test_findings_core.py index 1005d81a1..347d7d28c 100644 --- a/tests/test_findings_core.py +++ b/tests/test_findings_core.py @@ -1,6 +1,9 @@ +import pytest + from appguardrail_core.findings import (finding_sort_key, is_deploy_blocking, normalize_finding, normalize_findings, - safe_report_snippet, severity_counts) + safe_report_snippet, severity_counts, + severities_at_or_above) class ExplosiveText: @@ -11,6 +14,27 @@ def __str__(self): raise RuntimeError("caller-controlled conversion failed") +class NonIterableReference: + def __str__(self): + return "fallback-reference" + + +class ExplosiveReferences: + def __iter__(self): + yield "preserved-reference" + raise RuntimeError("caller-controlled iteration failed") + + +class SecretText: + def __str__(self): + return "Authorization: ApiKey coerced-secret-value" + + +class SecretLine: + def __str__(self): + return "Authorization: ApiKey late-coercion-secret" + + def test_normalize_finding_adds_report_contract_defaults(): finding = normalize_finding( { @@ -107,6 +131,18 @@ def test_severity_counts_folds_unknown_values_into_info(): assert counts == {"CRITICAL": 1, "HIGH": 0, "WARNING": 0, "INFO": 3} +def test_finding_reference_and_severity_fallback_edges(): + fallback = normalize_finding({"references": NonIterableReference()}) + failed = normalize_finding({"references": ExplosiveText()}) + partial = normalize_finding({"references": ExplosiveReferences()}) + + assert fallback["references"] == ("fallback-reference",) + assert failed["references"] == () + assert partial["references"] == ("preserved-reference",) + assert severities_at_or_above("not-a-severity") == {"CRITICAL", "HIGH"} + assert severities_at_or_above("WARNING") == {"CRITICAL", "HIGH", "WARNING"} + + def test_is_deploy_blocking_uses_context_and_case_insensitive_severity(): assert is_deploy_blocking({"severity": "critical", "context": "app-code"}) assert is_deploy_blocking({"severity": "HIGH"}) @@ -130,3 +166,300 @@ def test_finding_sort_key_orders_by_deploy_severity_then_category_and_rule(): def test_safe_report_snippet_trims_without_changing_short_text(): assert safe_report_snippet("short evidence") == "short evidence" assert safe_report_snippet("x" * 410, max_len=20) == "x" * 20 + "\n...[truncated]" + + +def test_normalize_finding_suppresses_imported_secret_evidence(): + synthetic_secret = "synthetic-secret-value-for-regression-only" + + finding = normalize_finding( + { + "rule_id": "external-finding-42", + "category": "secrets", + "snippet": f'password="{synthetic_secret}"', + } + ) + + assert finding["snippet"] == "[REDACTED: sensitive match suppressed]" + assert synthetic_secret not in finding["snippet"] + + +def test_normalize_finding_suppresses_sensitive_rule_with_wrong_category(): + synthetic_secret = "synthetic-secret-value-for-regression-only" + + finding = normalize_finding( + { + "rule_id": "external-hardcoded-credential", + "category": "misconfig", + "snippet": synthetic_secret, + } + ) + + assert finding["snippet"] == "[REDACTED: sensitive match suppressed]" + assert synthetic_secret not in finding["snippet"] + + +def test_normalize_finding_redacts_obvious_secret_in_nonsecret_category(): + synthetic_token = "synthetic-bearer-token-for-regression-only" + + finding = normalize_finding( + { + "rule_id": "external-http-header", + "category": "misconfig", + "snippet": f"Authorization: Bearer {synthetic_token}", + } + ) + + assert finding["snippet"] == "Authorization: [REDACTED]" + assert synthetic_token not in finding["snippet"] + + +def test_normalize_finding_sanitizes_all_emitted_and_extension_fields(): + synthetic_token = "ghp_" + "a" * 24 + finding = normalize_finding( + { + "rule_id": "external-finding-42", + "category": "misconfig", + "message": f"Observed token={synthetic_token}", + "file": f"evidence/{synthetic_token}.txt", + "remediation": f"Rotate password='{synthetic_token}'", + "verification": f"Reject Authorization: Bearer {synthetic_token}", + "references": [f"https://example.invalid/{synthetic_token}"], + "raw_evidence": {"body": synthetic_token}, + "provider_metadata": { + "authorization": synthetic_token, + "snippet": "opaque-unknown-secret", + synthetic_token: "present", + }, + "raw": "opaque-raw-value", + "raw_response": "opaque-response-value", + "credentials": ["opaque-credential-value"], + "credentialData": {"value": "opaque-credential-data"}, + "apiKey": 123456789, + } + ) + + assert synthetic_token not in repr(finding) + assert finding["raw_evidence"] == "[REDACTED]" + assert finding["provider_metadata"]["authorization"] == "[REDACTED]" + assert finding["provider_metadata"]["snippet"] == "[REDACTED]" + assert synthetic_token not in finding["provider_metadata"] + assert "[REDACTED]" in finding["provider_metadata"] + assert finding["raw"] == "[REDACTED]" + assert finding["raw_response"] == "[REDACTED]" + assert finding["credentials"] == "[REDACTED]" + assert finding["credentialData"] == "[REDACTED]" + assert finding["apiKey"] == "[REDACTED]" + + +@pytest.mark.parametrize( + "field_name", + ( + "apiKeys", + "api_keys", + "apiKeyValue", + "api_key_value", + "privateKeys", + "private_keys", + "privateKeyData", + "private_key_pem", + "accessKeyId", + "access_key_id", + "passwords", + ), +) +def test_normalize_finding_redacts_plural_sensitive_fields(field_name): + finding = normalize_finding( + { + "rule_id": "external-finding-42", + "category": "misconfig", + field_name: ["opaque-production-credential"], + } + ) + + assert finding[field_name] == "[REDACTED]" + + +def test_normalize_finding_redacts_multiline_quoted_assignment(): + finding = normalize_finding( + { + "rule_id": "external-finding-42", + "category": "misconfig", + "message": "password='first-secret-line\nsecond-secret-line'", + } + ) + + assert finding["message"] == "password=[REDACTED]" + + +@pytest.mark.parametrize("delimiter", ("'''", '\"\"\"', "`")) +def test_normalize_finding_redacts_multiline_delimited_assignment(delimiter): + message = ( + f"private_key = {delimiter}-----BEGIN PRIVATE KEY-----\n" + f"OPAQUEKEYBODY\n-----END PRIVATE KEY-----{delimiter}" + ) + + finding = normalize_finding({"message": message}) + + assert finding["message"] == "private_key = [REDACTED]" + + +@pytest.mark.parametrize( + "message", + ( + r'password="""first-secret\""" second-secret"""', + r"password='''first-secret\''' second-secret'''", + ), +) +def test_normalize_finding_honors_escaped_triple_quote(message): + finding = normalize_finding({"message": message}) + + assert finding["message"] == "password=[REDACTED]" + + +@pytest.mark.parametrize("scope", ("proj", "svcacct")) +def test_normalize_finding_redacts_scoped_provider_token(scope): + token = f"sk-{scope}-" + "a" * 24 + + finding = normalize_finding({"message": f"Observed {token}"}) + + assert token not in finding["message"] + + +def test_normalize_finding_validates_line_without_late_string_coercion(): + finding = normalize_finding({"line": SecretLine()}) + + assert finding["line"] == 1 + assert "late-coercion-secret" not in repr(finding) + + +@pytest.mark.parametrize( + ("raw_line", "expected"), + (("7", 7), ("0", 1), ("9" * 21, 1), ("seven", 1), (True, 1)), +) +def test_normalize_finding_validates_line_number(raw_line, expected): + assert normalize_finding({"line": raw_line})["line"] == expected + + +def test_normalize_finding_preserves_values_when_secret_keys_collide(): + first_token = "ghp_" + "a" * 24 + second_token = "ghp_" + "b" * 24 + third_token = "ghp_" + "c" * 24 + + finding = normalize_finding( + { + "rule_id": "external-finding-42", + "category": "misconfig", + "provider_metadata": { + first_token: "first nonsecret value", + second_token: "second nonsecret value", + third_token: "third nonsecret value", + }, + } + ) + + assert finding["provider_metadata"] == { + "[REDACTED]": "first nonsecret value", + "[REDACTED]#2": "second nonsecret value", + "[REDACTED]#3": "third nonsecret value", + } + + +def test_normalize_finding_redacts_text_introduced_by_coercion(): + finding = normalize_finding( + { + "rule_id": "external-finding-42", + "category": "misconfig", + "message": SecretText(), + } + ) + + assert "coerced-secret-value" not in finding["message"] + + +def test_safe_report_snippet_bounds_text_before_pattern_scanning(monkeypatch): + observed_bounds = [] + + def record_bounds(text, *, visible_length): + observed_bounds.append((len(text), visible_length)) + return text[:visible_length] + + monkeypatch.setattr( + "appguardrail_core.findings.redact_sensitive_prefix", record_bounds + ) + + safe_report_snippet("x" * 10_000, max_len=20) + + assert observed_bounds == [(148, 20)] + + +def test_safe_report_snippet_redacts_token_crossing_truncation_boundary(): + token = "ghp_" + "a" * 24 + snippet = "x" * 380 + " " + token + + rendered = safe_report_snippet(snippet, max_len=400) + + assert "ghp_" not in rendered + assert token not in rendered + assert rendered.endswith("\n...[truncated]") + + +def test_safe_report_snippet_never_pulls_lookahead_into_output(): + token = "ghp_" + "a" * 24 + opaque_tail = "opaque-value-beyond-cutoff" + snippet = token + " " + "x" * (400 - len(token) - 1) + opaque_tail + + rendered = safe_report_snippet(snippet, max_len=400) + + assert opaque_tail not in rendered + assert "opaque-value" not in rendered + assert rendered.endswith("\n...[truncated]") + + +def test_safe_report_snippet_merges_overlapping_secret_spans(): + token = "ghp_" + "a" * 24 + snippet = f"Authorization: Bearer {token}" + "x" * 500 + + rendered = safe_report_snippet(snippet, max_len=40) + + assert rendered == "Authorization: [REDACTED]\n...[truncated]" + + +def test_nonsecret_provider_rules_preserve_benign_evidence(): + for rule_id in ( + "github-actions-mutable-branch-writer", + "stripe-price-from-client", + "aws-public-bucket", + "google-oauth-redirect", + "insecure-random-security-token", + ): + finding = normalize_finding( + { + "rule_id": rule_id, + "category": "misconfig", + "snippet": "benign configuration evidence", + } + ) + + assert finding["snippet"] == "benign configuration evidence" + + +@pytest.mark.parametrize( + "rule_id", + ( + "external-api-token-exposure", + "credential-leak", + "password-disclosure", + "private-key-checked-in", + "jwt-exposure", + ), +) +def test_sensitive_rule_semantics_suppress_opaque_snippet(rule_id): + finding = normalize_finding( + { + "rule_id": rule_id, + "category": "misconfig", + "snippet": "opaque-value-without-known-provider-format", + } + ) + + assert finding["snippet"] == "[REDACTED: sensitive match suppressed]" diff --git a/tests/test_issueops_core.py b/tests/test_issueops_core.py index 9ee2f5d8b..5a239d8ab 100644 --- a/tests/test_issueops_core.py +++ b/tests/test_issueops_core.py @@ -1,3 +1,5 @@ +import re + import pytest from appguardrail_core import issueops @@ -52,6 +54,19 @@ def test_timeout_diagnosis_adds_timeout_specific_next_step(): assert "runner capacity and configured timeouts" in body +@pytest.mark.parametrize( + ("conclusion", "expected"), + [ + ("cancelled", "Determine who or what cancelled the run"), + ("action_required", "Have an authorized maintainer approve"), + ], +) +def test_diagnosis_adds_terminal_state_specific_next_step(conclusion, expected): + body = issueops.diagnosis(_finding(conclusion=conclusion)) + + assert expected in body + + def finding(**overrides): base = { "repo": "ContextualWisdomLab/naruon", @@ -177,12 +192,127 @@ def test_redact_handles_multiple_assignments_without_consuming_field_names(): ) +@pytest.mark.parametrize( + ("secret_text", "forbidden"), + [ + ( + '{"Authorization": "Bearer syntheticCredentialValue123456"}', + "syntheticCredentialValue123456", + ), + ( + "client_secret='syntheticCredentialValue123456'", + "syntheticCredentialValue123456", + ), + ( + "access_token=syntheticCredentialValue123456", + "syntheticCredentialValue123456", + ), + ( + "aws_secret_access_key=syntheticCredentialValue123456", + "syntheticCredentialValue123456", + ), + ("AWS_ACCESS_KEY_ID=AKIA" + "A" * 16, "AKIA" + "A" * 16), + ( + "AWS_SECRET_ACCESS_KEY_ID=syntheticCredentialValue123456", + "syntheticCredentialValue123456", + ), + ( + "Authorization: ApiKey opaque-secret-value trailing-parameter", + "opaque-secret-value", + ), + ( + "Authorization: Digest username=demo,response=opaque-secret-value", + "opaque-secret-value", + ), + ("AKIA" + "A" * 16, "AKIA" + "A" * 16), + ("AIza" + "A" * 35, "AIza" + "A" * 35), + ("xoxb-" + "a" * 20, "xoxb-" + "a" * 20), + ("sk_live_" + "a" * 24, "sk_live_" + "a" * 24), + ("sk-ant-" + "a" * 24, "sk-ant-" + "a" * 24), + ("sk-proj-" + "a" * 24, "sk-proj-" + "a" * 24), + ("sk-svcacct-" + "a" * 24, "sk-svcacct-" + "a" * 24), + ("SG." + "a" * 20 + "." + "b" * 20, "SG." + "a" * 20), + ("npm_" + "a" * 24, "npm_" + "a" * 24), + ("pypi-" + "a" * 24, "pypi-" + "a" * 24), + ], +) +def test_redact_covers_supported_obvious_credential_forms(secret_text, forbidden): + assert forbidden not in issueops.redact(secret_text) + + +def test_issueops_secret_re_alias_remains_effective(monkeypatch): + monkeypatch.setattr(issueops, "SECRET_RE", [re.compile(r"(custom=)[^\s]+")]) + + assert issueops.redact("custom=opaque-value") == "custom=[REDACTED]" + + +@pytest.mark.parametrize( + "benign_text", + [ + "AKIA-short-example", + "npm_package_name=appguardrail", + "client_secretary=assigned", + "Authorization guidance belongs in the security handbook", + "sk-not-a-secret-but-long-enough-for-regex", + ], +) +def test_redact_preserves_near_miss_text(benign_text): + assert issueops.redact(benign_text) == benign_text + + def test_redact_fails_closed_for_unterminated_quoted_secret(): assert issueops.redact("password='secret value without closing quote") == ( "password=[REDACTED]" ) +def test_redact_consumes_multiline_quoted_secret(): + log = "password='first-secret-line\nsecond-secret-line'" + + assert issueops.redact(log) == "password=[REDACTED]" + + +@pytest.mark.parametrize("delimiter", ("'''", '\"\"\"', "`")) +def test_redact_consumes_multiline_delimited_secret(delimiter): + log = ( + f"private_key = {delimiter}-----BEGIN PRIVATE KEY-----\n" + f"OPAQUEKEYBODY\n-----END PRIVATE KEY-----{delimiter}" + ) + + assert issueops.redact(log) == "private_key = [REDACTED]" + + +@pytest.mark.parametrize( + "log", + ( + r'password="""first-secret\""" second-secret"""', + r"password='''first-secret\''' second-secret'''", + ), +) +def test_redact_honors_escaped_triple_quote(log): + assert issueops.redact(log) == "password=[REDACTED]" + + +def test_log_compression_and_marker_fallback_edges(): + assert issueops.compress_log("") == "(no job log returned)" + assert issueops.compress_log("plain one\nplain two", max_lines=1) == ( + "plain two\n...[compressed]" + ) + + many_failures = "\n".join(f"error failure {index}" for index in range(30)) + compressed = issueops.compress_log(many_failures, max_lines=2, max_chars=12) + assert "...[truncated]" in compressed + assert "...[compressed]" in compressed + + assert issueops.parse_marker(None) == {"seen": []} + assert issueops.parse_marker( + f"{issueops.MARKER_PREFIX} invalid-json {issueops.MARKER_SUFFIX}" + ) == {"seen": []} + inserted = issueops.replace_marker("existing body", "owner/repo", "CodeQL", {"1:2"}) + assert inserted.startswith(issueops.MARKER_PREFIX) + assert inserted.endswith("existing body") + + def test_marker_body_and_replacement_round_trip(): item = finding() body = issueops.issue_body(item, {issueops.seen_key(item)}) diff --git a/tests/test_reports_core.py b/tests/test_reports_core.py index 97de050a1..9b26ed693 100644 --- a/tests/test_reports_core.py +++ b/tests/test_reports_core.py @@ -1,3 +1,5 @@ +import pytest + from appguardrail_core.reports import (ReportContext, render_agency_report, render_buyer_diligence_report, render_fix_pack, @@ -80,6 +82,71 @@ def test_render_buyer_diligence_report_truncates_long_snippets(): assert "x" * 450 not in report +def test_render_report_does_not_reemit_imported_secret_snippet(): + synthetic_secret = "synthetic-secret-value-for-regression-only" + + report = render_buyer_diligence_report( + [ + { + "rule_id": "external-finding-42", + "severity": "CRITICAL", + "category": "secrets", + "message": "External scanner located a credential.", + "file": "settings.py", + "line": 4, + "snippet": f'password="{synthetic_secret}"', + } + ], + ReportContext(generated_at="2026-10-09T00:00:00Z"), + ) + + assert "[REDACTED: sensitive match suppressed]" in report + assert synthetic_secret not in report + + +def test_render_report_does_not_coerce_untrusted_line_object(): + class LateSecretLine: + def __str__(self): + return "Authorization: ApiKey late-coercion-secret" + + report = render_buyer_diligence_report( + [{"rule_id": "external-finding-42", "line": LateSecretLine()}], + ReportContext(generated_at="2026-10-09T00:00:00Z"), + ) + + assert "late-coercion-secret" not in report + assert "`n/a:1`" in report + + +@pytest.mark.parametrize( + "renderer", + ( + render_buyer_diligence_report, + render_founder_friendly_report, + render_agency_report, + render_fix_pack, + ), +) +def test_every_report_redacts_secrets_outside_snippet(renderer): + synthetic_token = "ghp_" + "b" * 24 + + report = renderer( + [ + { + "rule_id": "external-finding-42", + "severity": "CRITICAL", + "category": "misconfig", + "message": f"Observed token={synthetic_token}", + "remediation": f"Rotate password='{synthetic_token}'", + "raw_evidence": synthetic_token, + } + ], + ReportContext(generated_at="2026-10-09T00:00:00Z"), + ) + + assert synthetic_token not in report + + def test_render_buyer_diligence_report_handles_empty_findings(): report = render_buyer_diligence_report( [], diff --git a/tests/test_sarif.py b/tests/test_sarif.py index 742082030..bafa79702 100644 --- a/tests/test_sarif.py +++ b/tests/test_sarif.py @@ -1,5 +1,7 @@ """Tests for SARIF 2.1.0 output (appguardrail_core.sarif).""" +import json + from appguardrail_core.sarif import findings_to_sarif FINDINGS = [ @@ -72,3 +74,21 @@ def test_empty_findings_valid(): run = findings_to_sarif([])["runs"][0] assert run["results"] == [] assert run["tool"]["driver"]["rules"] == [] + + +def test_sarif_does_not_emit_secret_from_untrusted_finding_fields(): + synthetic_token = "ghp_" + "c" * 24 + + sarif = findings_to_sarif( + [ + { + "severity": "HIGH", + "rule_id": "external-finding-42", + "message": f"Observed token={synthetic_token}", + "remediation": f"Rotate password='{synthetic_token}'", + "raw_evidence": synthetic_token, + } + ] + ) + + assert synthetic_token not in json.dumps(sarif)