fix(data-layer): redact loop event/status/decision outside their allowed sets - #64
Open
diazMelgarejo wants to merge 1 commit into
Open
Conversation
…wed sets normalize_loop_event() in .agent/tools/data_layer_export.py copies entry["event"], entry["status"], and entry["decision"] straight into the exported "action" and "result" fields with no validation. Those three keys are supposed to come from a small, supervisor-controlled finite set -- harness_manager/loops/runner.py only ever writes 10 distinct event names, 10 status names, and 3 decision names to runtime/loops/events.jsonl -- but nothing enforced that on the export path, so any row with an unexpected value for those fields (a bug upstream, a hand-edited events.jsonl, or a future loop kind that doesn't yet exist) would flow the raw string straight through into the dashboard/analytics surface. That matters more here than it would in a generic ETL script: this repo's own design intent for the loop event journal is that it's content-free by construction. task/prompt/command/output are excluded from events.jsonl entirely for exactly this reason -- see the existing test_exports_privacy_safe_loop_events_and_quality_counts coverage, which already asserts a "task" field never makes it into the export. event/status/decision were the one gap in that whitelist discipline: a closed, small vocabulary in the writer, treated as open text on the read side. Add VALID_LOOP_EVENTS/VALID_LOOP_STATUSES/VALID_LOOP_DECISIONS (mirrored directly from runner.py's own literals) and a small _allowed_or_unknown() helper. Anything outside the allowed set redacts to "unknown" -- the same fallback already used everywhere else in this file for missing or unrecognized values, so this isn't introducing a new convention, just applying the existing one to a field that was skipped. Adds one regression test alongside the existing loop-event privacy test, verified to fail against the pre-fix code (a script tag and a free-text string both land in the exported JSONL verbatim without the fix) and pass with it.
Contributor
Author
|
Related: #65 (two smaller, unrelated fixes found in the same repo while working on this one). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The design promise this closes a gap in
docs/specs/v0.20-agentic-turn.md(this repo's own current spec, currently atmasterHEAD) states the loop event journal is meant to be content-free by construction: task, prompt, command, and output are excluded fromruntime/loops/events.jsonlfor exactly this reason, on the theory that two append-only journals under.agent/with opposite privacy guarantees means the weaker one becomes the leak..agent/tools/data_layer_export.pyalready honors that promise for the fields it was built to exclude —test_exports_privacy_safe_loop_events_and_quality_countsasserts a"task"value never makes it into the export.But the journal's remaining three fields —
event,status,decision— were never given the same treatment. They are supposed to be a closed, small vocabulary:harness_manager/loops/runner.pywrites exactly 10 distinct event names, 10 status names, and 3 decision names, full stop — there's no code path that emits anything else.normalize_loop_event(), though, treats them as open text:str(entry.get("event") or "loop_event")copies whatever string is there straight into the exportedactionfield, and thestatus/decisionunion does the same forresult. The write side is a closed enum; the read side trusted it as free-form input. That gap is the one place the content-free-journal guarantee this repo explicitly designed for was left unenforced.Concretely, this means any row with an
event/status/decisionvalue outside the real set — from a future bug in the supervisor, a hand-editedevents.jsonl, or a loop kind that doesn't exist yet — flows verbatim into the dashboard/analytics export with zero validation. Nothing catastrophic on its own, but it's exactly the kind of small, structural gap that turns into a real leak the moment something upstream of it goes wrong, which is the whole reason the journal was designed to be content-free in the first place.The fix
Mirror the actual literals
runner.pywrites into three module-level sets —VALID_LOOP_EVENTS,VALID_LOOP_STATUSES,VALID_LOOP_DECISIONS— and a small_allowed_or_unknown()helper that redacts anything outside the allowed set to"unknown". That's not a new convention:"unknown"is already the fallback this file uses everywhere else for missing or unrecognized values (pii_level,result,harness, and half a dozen other fields all do the same thing already). This fix just extends the pattern to the two fields that had skipped it.Tests
Added
test_redacts_loop_event_status_and_decision_outside_the_allowed_setsnext to the existing loop-event privacy test. Verified in both directions before opening this PR: it fails against the pre-fix code (a<script>tag and a free-text "leaked prompt" string both land in the exported JSONL verbatim), and passes with the fix applied. Full suite:pytest tests/test_data_layer_export.py— 8 passed.Scope
One file, one function, plus its test. No schema changes, no new dependencies, no behavior change for any well-formed event.
Note
Redact unrecognized loop event, status, and decision values in data layer export
VALID_LOOP_EVENTS,VALID_LOOP_STATUSES,VALID_LOOP_DECISIONS) in data_layer_export.py to enumerate permissible values for loop fields.normalize_loop_eventnow passes event, status, and decision values through a new_allowed_or_unknownhelper, replacing unrecognized values with'unknown'instead of copying arbitrary text into the export.'unknown'inagent-events.jsonlrather than the original string.Macroscope summarized 5a2724e.