Skip to content

[Security] Unredacted MuralError text printed to stderr by mural CLI main() #2756

Description

@jkim323

Summary

The top-level MuralError handler in the mural CLI main() writes the exception text to stderr without passing it through _redact. Several MuralAPIError messages are constructed directly from raw OAuth token-endpoint and asset-upload response bodies, so a token-refresh failure can print credential material in clear text.

This is the same failure class as #2460, but at a different sink. It is not covered by CodeQL alert 388 (already fixed), and it is not currently detected by any scanner.

Evidence

The unredacted sink, in scripts/mural/__init__.py:

except MuralError as exc:
    print(f"error: {exc}", file=sys.stderr)      # <-- not redacted
    return 1
except Exception as exc:  # noqa: BLE001
    print(f"internal error: {_redact(repr(exc))}", file=sys.stderr)   # <-- redacted

The broad Exception fallback redacts. The narrower MuralError handler — the one that actually carries API response bodies — does not.

A second MuralError handler earlier in the same file has the same defect:

except MuralError as exc:
    print(str(exc), file=sys.stderr)      # <-- L1195, not redacted
    return EXIT_FAILURE

Both sites must be fixed. Repairing only L1249 leaves the vulnerability reachable through L1195.

Tainted messages reach that handler from scripts/mural/_transport.py:

Line Construction Carries
L270 MuralAPIError(status, "TOKEN_INVALID_JSON", text) Raw token-endpoint response body
L315-L317 MuralAPIError(exc.code, "REFRESH_FAILED", text) Raw refresh-endpoint error body
L319 MuralAPIError(status, "REFRESH_FAILED", json.dumps(data)) Full parsed refresh payload
L603 MuralAPIError(status, "ASSET_UPLOAD_FAILED", payload) Azure Blob response (SAS query strings)

The clearest demonstration is the refresh path, where the same string is handled two different ways:

except urllib.error.HTTPError as exc:
    text = _read_response_body(exc).decode("utf-8", errors="replace")
    _emit(f"refresh failed: HTTP {exc.code} {text}", level=logging.ERROR)   # redacted via _emit
    raise MuralAPIError(exc.code, "REFRESH_FAILED", text or "refresh failed") from exc

_emit routes through _redact. The re-raise does not, and main() then prints it verbatim. So the defense holds on the logging path and is bypassed on the exception path.

Why this matters

mural-log-hygiene.instructions.md states that _redact "only protects log output that is actually routed through it. Bare LOGGER.* and print(*) sites bypass it." This is exactly such a site, on a path that handles OAuth refresh tokens and client secrets. The code contradicts its own documented contract.

Why scanners missed it

py/clear-text-logging-sensitive-data traced taint through a local dict in _cli_auth.py (alert 388) but does not follow taint across module boundaries through exception construction and re-raise into a top-level handler. Detection here requires either manual review or a custom query.

Proposed fix

  1. Wrap the output of both MuralError handlers (L1195 and L1249) in _redact, matching the adjacent Exception handler.
  2. Audit the other print(json.dumps(envelope), file=sys.stderr) handlers in main() and route them through a redacting helper for uniformity.
  3. Prefer not embedding raw response bodies in exception messages at all in _transport.py; carry a status/code plus a redacted excerpt.
  4. Add a regression test alongside tests/test_redaction.py asserting that a MuralAPIError carrying token-shaped material is masked when surfaced by main().

Acceptance criteria

  • MuralError output is redacted at both handler sites (L1195 and L1249)
  • Remaining stderr JSON envelopes in main() use a redacting path
  • _transport.py no longer embeds raw token/asset response bodies in exception messages
  • Regression test covers the main() exception sink
  • Reviewed and validated by a qualified human reviewer

Related

Metadata

Metadata

Labels

bugSomething isn't workingexperimentalpriority-2High priority, address soonsecuritySecurity-related changes or concernsskillsCopilot skill packages (SKILL.md)

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions