Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion hermes_cli/kanban_survivor.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,21 @@ def _git(repo, *args, env=None, check=True):
capture_output=True, timeout=30, env=env,
)
if check and result.returncode:
# Git stderr can contain credential-bearing remote URLs. Do not persist it.
# Git stderr can contain credential-bearing remote URLs, so it is never
# persisted: the raised message stays constant and the detail goes to
# the log, redacted by the same helper that guards an echoed claim.
#
# Without this, every occurrence costs an attribution pass. A failure
# that is purely environmental (t_169d6e46: a concurrent pytest session
# deleting this repo's tmp_path, so git exits 128 "cannot change to
# '<path>': No such file or directory") is indistinguishable from a real
# capture defect once it reaches the caller as a bare
# "survivor_unavailable: git inspection failed".
log.warning(
"kanban survivor: git %s failed rc=%s in %s: %s",
args[0] if args else "?", result.returncode, _ext.redact(str(repo)),
_ext.redact(result.stderr.decode("utf-8", "replace").strip()),
)
raise SurvivorUnavailable("survivor_unavailable: git inspection failed")
return result

Expand Down
51 changes: 51 additions & 0 deletions tests/hermes_cli/test_kanban_survivor_authority.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,3 +205,54 @@ def test_mined_pr_corroborated_by_the_cards_own_metadata_is_accepted(board, remo
saved = kb.latest_run(board, tid).metadata["survivor"]
assert saved["kind"] == "ref"
assert saved["refs"][0]["sha"] == MERGE


# --- t_169d6e46: a swallowed git failure must be diagnosable ----------------

def test_git_failure_logs_returncode_and_stderr_without_persisting_them(tmp_path, caplog):
"""`survivor_unavailable: git inspection failed` discards WHY it failed.

That message is identical for a real capture defect and for a purely
environmental fault (t_169d6e46: a concurrent pytest session deleting this
repo's tmp_path, so git exits 128 `cannot change to '<path>'`). Telling them
apart cost a full attribution pass per occurrence. The returncode and stderr
must reach the LOG; the raised message must stay constant and credential-free,
because it is persisted to held_reason, the event log and stderr.
"""
import hermes_cli.kanban_survivor as survivor

missing = tmp_path / "vanished" # never created: git exits 128
with caplog.at_level("WARNING"):
with pytest.raises(survivor.SurvivorUnavailable) as excinfo:
survivor._git(missing, "status", "--porcelain")

# The persisted surface is unchanged — 5 open PRs key on this string.
assert str(excinfo.value) == "survivor_unavailable: git inspection failed"
# ...and the diagnostic that distinguishes environment from defect is logged.
assert "rc=128" in caplog.text
assert "status" in caplog.text
assert "No such file or directory" in caplog.text


def test_git_failure_log_redacts_credentials_from_stderr(tmp_path, caplog, monkeypatch):
"""Git stderr can carry a credential-bearing remote URL; the log must not.

Teeth for the test above: an unredacted `log.warning(stderr)` would satisfy
every assertion there while leaking a PAT into the logfile.
"""
import subprocess as sp

import hermes_cli.kanban_survivor as survivor

leaky = f"fatal: could not read from https://x-access-token:{SECRET}@github.com/a/b.git\n"
monkeypatch.setattr(
survivor.subprocess, "run",
lambda *a, **k: sp.CompletedProcess(a[0], 128, b"", leaky.encode()),
)
with caplog.at_level("WARNING"):
with pytest.raises(survivor.SurvivorUnavailable):
survivor._git(tmp_path, "fetch")

assert SECRET not in caplog.text
assert SECRET not in str(caplog.records[-1].getMessage())
assert "rc=128" in caplog.text # still diagnosable
Loading