Skip to content

fix(redaction): keep Actions group markers from wiping JSON evidence - #1031

Closed
cursor[bot] wants to merge 2 commits into
mainfrom
cursor/bc-ec4acde9-05b0-4651-ae7f-5bb444f95c2f-b548
Closed

fix(redaction): keep Actions group markers from wiping JSON evidence#1031
cursor[bot] wants to merge 2 commits into
mainfrom
cursor/bc-ec4acde9-05b0-4651-ae7f-5bb444f95c2f-b548

Conversation

@cursor

@cursor cursor Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Next action

Merge this successor instead of #929. #929's span finder treats every [ as JSON, so a GitHub Actions ##[group] marker plus a later pretty-printed password object fail-closes the entire job log to [REDACTED].

What changed

  • Start a raw JSON span only after a plausible opener prefix, and score a failed parse only until the next plausible opener.
  • Keep ##[group] / prose [timeout] diagnostics visible while rewriting credential leaves in place.
  • Cite RFC 8259, ECMA-404, and ISO/IEC 21778 (APA 7) for duplicate-member layout, and pin the quality-gate operator prose.
  • Add a realistic Actions job-log fixture and a bracket-diagnostic control.

Verification

Focused redaction selection: 163 passed. Owned modules redact_sensitive_log.py, sandboxed_verify.py, and sandboxed_web_e2e.py report 100% statement and branch coverage (1,064 statements, 426 branches).

Acceptance

Do not merge #929 on the previous head. After this head is green, treat [REDACTED] as evidence suppression and rerun the exact-head sandbox redaction quality job before closing #908.

Open in Web View Automation 

seonghobae and others added 2 commits August 17, 2026 00:22
Replay unique #929 source onto current origin/main. Skip shared
ARCHITECTURE/CLAUDE/AGENTS trees and stale materialize files.
Treat only plausible JSON openers as span starts and score a failed parse
against the window until the next opener. A ##[group] marker or prose
[timeout] no longer fail-closes a later pretty-printed password object.
Cite RFC 8259, ECMA-404, and ISO/IEC 21778 for duplicate-member layout.

Co-authored-by: Seongho Bae <[email protected]>
@seonghobae
seonghobae marked this pull request as ready for review August 16, 2026 15:29
@seonghobae
seonghobae self-requested a review as a code owner August 16, 2026 15:29
@cursor

cursor Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor Author

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not merge this head. Same-author review cannot request changes here, so this is a COMMENT plus a successor that must land instead.

The ##[group] skip is real for contiguous spans, but it does not satisfy the claimed Actions job-log acceptance. collect_failed_check_evidence.sh pipes downloaded logs through this redactor after strip_ansi only. GitHub prefixes every line with YYYY-MM-DDTHH:MM:SS.nnnnnnnZ , including the JSON body. On that input, { after the timestamp space is a plausible opener, the next timestamp breaks the parse, the failed window runs to EOF, and _looks_like_sensitive_json_candidate sees "password": and fail-closes the entire log to [REDACTED]. I reproduced that wipe locally. The current fixture timestamps only the group/endgroup lines, so it cannot see the production path.

A line-start [INFO] that mentions "password": has the same wipe: [ is still a plausible array start. The [timeout] control uses retry [timeout] plus a later complete object, so its failed window never includes a sensitive key.

Next action: do not merge #929 or this head. Land the successor that accepts a per-line-timestamped pretty-printed password dump (group/status survive; only credential leaves become [REDACTED]) and keeps a line-start [INFO] ... "password": diagnostic from erasing a later complete object.

Open in Web View Automation 

Sent by Cursor Automation: Fix Issues

first = _credential("actions")
second = _credential("dup")
source = (
"2026-08-16T15:22:12.001Z ##[group]Runner\n"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixture is not a downloaded Actions job log. GitHub prefixes every line with YYYY-MM-DDTHH:MM:SS.nnnnnnnZ , including {, each member, and }. The current source only timestamps the group/endgroup lines, so { remains a contiguous span and the test cannot see the production wipe.

I ran redact_text on the same dump with a timestamp on every line. The result was a single [REDACTED] and ##[group]Runner disappeared. collect_failed_check_evidence.sh pipes those timestamped logs through this redactor after strip_ansi only.

Add a fixture that timestamps every line. Accept only if group/status text survive and only the credential leaves become [REDACTED].

node, end = _raw_json_parse_value(text, start, 0, budget)
except _RawJsonError:
window_end = _next_plausible_raw_json_start(text, start + 1)
if _looks_like_sensitive_json_candidate(text[start:window_end]):

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A failed { opener still fail-closes the entire buffer when the window contains "password":. That is correct for a truncated object, but it is also what a real Actions job log produces: { after the timestamp space is a plausible start, the next line is another timestamp, parse fails, and the window runs to EOF because later [ in ##[endgroup] is not plausible (# is not a prefix).

The group-marker [ skip does not help here. The wipe is now triggered by the timestamp-split {, not by ##[group].

Treat a runner timestamp as line metadata the span parser can skip, then map replacements back onto the original source. Do not keep scoring a timestamp-interrupted pretty-printed object as one sensitive candidate window.

"""Return whether a brace or bracket can start a top-level JSON span."""
if index == 0:
return True
return text[index - 1] in RAW_JSON_SPAN_PREFIXES

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is a plausible prefix, so a line-start [INFO] / [ERROR] / [command] is still treated as a JSON array opener. I confirmed [INFO] schema requires "password": string {"status": "ok"} fail-closes the whole text to [REDACTED].

The [timeout] control uses retry [timeout] plus a later complete object, so the failed window is only [timeout] and does not include a sensitive key. That misses this path.

Treat [ as an opener only when the next significant character can start a JSON value (", digit, -, {, [, true, false, null, or ]). Identifier brackets such as [INFO] and [timeout] must not open a candidate window.

@seonghobae

Copy link
Copy Markdown
Contributor

Superseded by #1038. This head still fail-closes a downloaded per-line-timestamped pretty-printed password dump and a line-start [INFO] diagnostic that names "password":. Do not merge. Close this PR and land #1038 after independent review and exact-head required checks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants