mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-08 22:22:52 +00:00
fix(eval): redact the token from the one failure line that now reaches CI
`ManagedProcessError.__str__` embeds up to 1000 raw bytes of stderr_tail (process_control.py:72-73), and run_cell printed it verbatim. That line was inert until this branch: nothing ever printed the sweep subprocess's stdout, on success or failure. `echo_stdout` streams it live into the job log, so the print became a sink — and the only one of its kind here that skipped `redact_text`, which results.jsonl and every transcript already apply to exactly this field, for exactly this reason. GitHub masks the registered secret, but masking only catches that literal value; it is not the guarantee the other sinks have. The test drives a ManagedProcessError carrying the token in stderr_tail and asserts it never reaches stdout — verified to fail without the fix. Also from the same pass: bind `result_indexes[0]` once in run_claude, and give the workflow contract test a `findStep` helper instead of five copies of the same `steps.find` predicate.
This commit is contained in:
parent
8076b98fcc
commit
b5bcdb6c48
4 changed files with 47 additions and 13 deletions
|
|
@ -516,6 +516,33 @@ def test_run_cell_records_an_expected_failure_and_still_removes_its_clone(monkey
|
|||
assert removed == [tmp_path / "clone"]
|
||||
|
||||
|
||||
def test_run_cell_redacts_the_auth_token_from_the_failure_it_prints(monkeypatch, tmp_path, capsys):
|
||||
_stub_cell_dependencies(monkeypatch, tmp_path)
|
||||
secret = "sk-ant-not-a-real-key"
|
||||
|
||||
def explode(*_args, **_kwargs):
|
||||
raise ManagedProcessError(
|
||||
["claude"],
|
||||
ManagedProcessResult(
|
||||
state="exited",
|
||||
returncode=1,
|
||||
stdout_tail="",
|
||||
stderr_tail=f"ANTHROPIC_API_KEY={secret}",
|
||||
duration_s=1.0,
|
||||
),
|
||||
)
|
||||
|
||||
monkeypatch.setattr(runner, "run_arm", explode)
|
||||
context = _cell_context(tmp_path)
|
||||
context.args.auth_token = secret
|
||||
# ManagedProcessError stringifies up to 1000 raw bytes of stderr_tail, and
|
||||
# this line streams live into the CI log now that the sweep's stdout is
|
||||
# echoed. results.jsonl already redacts the same field.
|
||||
runner.run_cell(context, 0, "workflow")
|
||||
|
||||
assert secret not in capsys.readouterr().out
|
||||
|
||||
|
||||
def test_run_cell_lets_an_unexpected_failure_escape_rather_than_scoring_it(monkeypatch, tmp_path):
|
||||
_, removed = _stub_cell_dependencies(monkeypatch, tmp_path)
|
||||
|
||||
|
|
|
|||
|
|
@ -905,7 +905,11 @@ def run_cell(ctx: TaskCellContext, run_idx: int, arm: str) -> dict[str, Any]:
|
|||
# report.md/promotion.json still get written.
|
||||
record = infra_error_record(exc)
|
||||
record["arm"] = arm
|
||||
print(f"[{task['id']}][{arm}][run {run_idx}] infra-error: {exc}")
|
||||
# ManagedProcessError carries up to 1000 raw bytes of stderr_tail, and
|
||||
# this line now streams live into the CI log (run_managed echoes the
|
||||
# sweep's stdout). Redact it like every other sink this data reaches.
|
||||
detail = redact_text(str(exc), [args.auth_token or ""])
|
||||
print(f"[{task['id']}][{arm}][run {run_idx}] infra-error: {detail}")
|
||||
finally:
|
||||
if worktree is not None and worktree.exists():
|
||||
try:
|
||||
|
|
|
|||
|
|
@ -438,19 +438,20 @@ def run_claude(
|
|||
result_indexes = [index for index, event in enumerate(events) if event.get("type") == "result"]
|
||||
if len(result_indexes) != 1:
|
||||
raise ValueError(f"expected exactly one final result event, observed {len(result_indexes)}")
|
||||
data = events[result_indexes[0]]
|
||||
result_index = result_indexes[0]
|
||||
data = events[result_index]
|
||||
# A session that used a background task drains its bookkeeping after
|
||||
# the final result event (`background_tasks_changed`, `task_updated`,
|
||||
# `task_notification` — all `type: "system"`), so the result is last
|
||||
# only among the events that carry evidence. `system` events hold no
|
||||
# tool_use/tool_result/usage payload and so cannot forge skill or cost
|
||||
# evidence; any other event after the result still fails closed.
|
||||
if any(event.get("type") != "system" for event in events[result_indexes[0] + 1 :]):
|
||||
if any(event.get("type") != "system" for event in events[result_index + 1 :]):
|
||||
raise ValueError("final result event is not the last event in the captured stream")
|
||||
# Nothing after the result is evidence. Cut the window here so that is
|
||||
# a property of what the evidence readers below can see, rather than an
|
||||
# assumption that a `system` event never carries a tool_use block.
|
||||
events = events[: result_indexes[0] + 1]
|
||||
events = events[: result_index + 1]
|
||||
except (UnicodeError, ValueError) as exc:
|
||||
event_stream_error = str(exc)
|
||||
data = {}
|
||||
|
|
|
|||
|
|
@ -34,8 +34,14 @@ const workflowDocument = load(workflow) as {
|
|||
|
||||
const evolveJob = workflowDocument.jobs?.evolve;
|
||||
|
||||
type WorkflowStep = NonNullable<NonNullable<typeof evolveJob>['steps']>[number];
|
||||
|
||||
function findStep(stepName: string): WorkflowStep | undefined {
|
||||
return evolveJob?.steps?.find(({ name }) => name === stepName);
|
||||
}
|
||||
|
||||
function stepRun(stepName: string): string {
|
||||
const step = evolveJob?.steps?.find(({ name }) => name === stepName);
|
||||
const step = findStep(stepName);
|
||||
return typeof step?.run === 'string' ? step.run : '';
|
||||
}
|
||||
|
||||
|
|
@ -75,9 +81,7 @@ describe('gitnexus skill-evolution workflow contract', () => {
|
|||
// The benchmark sandbox-copies node_modules from all three (tasks.scenarios.yaml).
|
||||
// The root tree was absent on the first real run because only the two subpackage
|
||||
// steps ran, so capture_task_dependency_binding aborted at task binding.
|
||||
const rootStep = evolveJob?.steps?.find(
|
||||
({ name }) => name === 'Install monorepo root dependencies',
|
||||
);
|
||||
const rootStep = findStep('Install monorepo root dependencies');
|
||||
expect(rootStep).toBeDefined();
|
||||
expect(rootStep).not.toHaveProperty('working-directory'); // installs at the repo root
|
||||
expect(String(rootStep?.run)).toContain('npm ci');
|
||||
|
|
@ -104,7 +108,7 @@ describe('gitnexus skill-evolution workflow contract', () => {
|
|||
|
||||
it('least-privileges the App token and gates the job on a protected Environment', () => {
|
||||
expect(evolveJob?.environment).toBe('gitnexus-evolution');
|
||||
const mint = evolveJob?.steps?.find(({ name }) => name === 'Mint GitHub App token');
|
||||
const mint = findStep('Mint GitHub App token');
|
||||
expect(mint?.with).toMatchObject({
|
||||
'client-id': expect.any(String),
|
||||
'permission-contents': 'write',
|
||||
|
|
@ -137,9 +141,7 @@ describe('gitnexus skill-evolution workflow contract', () => {
|
|||
// needs its own, strictly shorter budget: a step timeout only fails that
|
||||
// step, and the always() upload below still ships what it wrote.
|
||||
const jobBudget = evolveJob?.['timeout-minutes'];
|
||||
const loopStep = evolveJob?.steps?.find(
|
||||
({ name }) => name === 'Run the propose → benchmark → gate loop',
|
||||
);
|
||||
const loopStep = findStep('Run the propose → benchmark → gate loop');
|
||||
const stepBudget = loopStep?.['timeout-minutes'];
|
||||
expect(typeof jobBudget).toBe('number');
|
||||
expect(typeof stepBudget).toBe('number');
|
||||
|
|
@ -152,7 +154,7 @@ describe('gitnexus skill-evolution workflow contract', () => {
|
|||
});
|
||||
|
||||
it('uploads benchmark evidence unconditionally, on a path it addresses itself', () => {
|
||||
const upload = evolveJob?.steps?.find(({ name }) => name === 'Upload benchmark evidence');
|
||||
const upload = findStep('Upload benchmark evidence');
|
||||
// The sweep appends results.jsonl and transcripts as it goes, so a killed
|
||||
// generation still holds the evidence explaining why — and a path taken
|
||||
// from the killed step's outputs is exactly what would not be there.
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue