Skip to content

fix: harden Action log sanitization - #157

Merged
GrantBirki merged 6 commits into
openai:mainfrom
sylvesterkaczmarek:fix/sanitize-unicode-log-controls
Aug 18, 2026
Merged

fix: harden Action log sanitization#157
GrantBirki merged 6 commits into
openai:mainfrom
sylvesterkaczmarek:fix/sanitize-unicode-log-controls

Conversation

@sylvesterkaczmarek

@sylvesterkaczmarek sylvesterkaczmarek commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Harden ordinary Action log sanitization so it matches Fence's control-character policy and remains inside its fixed workflow-command byte bound after escaping.

Problem

There are two related gaps in action/log.cts on the base commit:

  1. sanitizeLogText removes C0 controls and normalizes CR/LF, but leaves C1 controls and Unicode line/paragraph separators intact even though Fence's structured-report path rejects them.
  2. workflowEscape asks sanitizeLogText to truncate to the 4096-byte debug limit and only then escapes % as %25. Escaping can therefore expand an already-maximal value well beyond the limit it was intended to enforce.

Both affect the same boundary: arbitrary diagnostic text should become one bounded, control-safe GitHub Actions log payload before emission.

Evidence / reproduction

  • On the base commit, sanitizeLogText removes C0 controls and U+007F, but its regular expression does not cover U+0080..U+009F, U+2028, or U+2029.
  • Fence's structuredRecord path already rejects C1 controls and U+2028/U+2029, so ordinary and structured logging enforce different control-character invariants.
  • A minimal control-character input such as before\u0085next\u2028tail retains those separators through the old ordinary sanitizer.
  • workflowEscape calls sanitizeLogText(value, 4096) before .replace(/%/g, "%25").
  • A minimal size reproduction is workflowEscape("%".repeat(4096)): the sanitized input is exactly 4096 bytes, but replacing every one-byte % with the three-byte %25 representation expands the returned workflow-command payload to roughly 12 KiB.
  • The updated action/log-sanitization.test.cts pins concrete C1 boundaries plus U+2028/U+2029, verifies the expected escaped output, and verifies a percent-heavy input remains at or below 4096 bytes after workflow escaping.
  • script/test-action-wrapper already runs this focused regression with the normal Action wrapper tests.

Printable Unicode remains unaffected. The second truncation is deliberately applied after workflow escaping so ordinary inputs keep the existing 4 KiB capacity while expansion-heavy values cannot exceed it.

Change

  • replace C1 controls (U+007F through U+009F) with _
  • replace Unicode line separator U+2028 and paragraph separator U+2029 with _
  • preserve existing CR/LF normalization
  • re-apply the 4096-byte bound after GitHub workflow-command escaping
  • add focused regressions for both control sanitization and post-escape expansion

This is logging-integrity hardening only; it does not alter Fence policy or enforcement behavior.

@sylvesterkaczmarek sylvesterkaczmarek changed the title fix: sanitize Unicode log control characters fix: harden Action log sanitization Aug 18, 2026

@GrantBirki GrantBirki left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed the log sanitization and byte-bound changes. The regression coverage looks good. Thanks!

@GrantBirki
GrantBirki merged commit e7aa8fc into openai:main Aug 18, 2026
39 checks passed
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