Skip to content

fix(unbound-hook): never let stdout encoding abort onboarding setup#153

Merged
vigneshsubbiah16 merged 1 commit into
stagingfrom
fix/unbound-hook-stdout-encoding
Jun 15, 2026
Merged

fix(unbound-hook): never let stdout encoding abort onboarding setup#153
vigneshsubbiah16 merged 1 commit into
stagingfrom
fix/unbound-hook-stdout-encoding

Conversation

@vigneshsubbiah16

@vigneshsubbiah16 vigneshsubbiah16 commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

What

unbound-hook setup aborts on Jamf's recurring check-in because the interpreter falls back to the ASCII stdout codec in that launchd context (no LANG/LC_* set). The first diagnostic line setup prints — the migration banner [migration] python->binary sweep — contained a -> arrow (U+2192), which raised UnicodeEncodeError and killed the run:

File ".../unbound_hook/setup_cmd.py", line 602, in run
UnicodeEncodeError: 'ascii' codec can't encode character '→' in position 80: ordinal not in range(128)
UNBOUND_INSTALL_FAILED step=setup code=1

Interactive sudo jamf policy runs inherit a UTF-8 locale and pass — which is why it slipped through canary and only showed up at fleet check-in. The output is purely diagnostic; a cosmetic write must never abort the install.

Fix

  • main() reconfigures stdout/stderr to UTF-8 (errors='replace') before dispatching any subcommand, so no print can raise UnicodeEncodeError. Also hardens the hook path, whose stdout carries UTF-8 JSON.
  • setup_cmd: swap the banner arrow for ASCII -> as defense in depth.

Tests

  • New test_setup_survives_ascii_stdout drives main(["setup", ...]) with ASCII-backed streams — reproduces the exact crash pre-fix, passes post-fix.
  • Verified on a real macOS interpreter under the field's stripped environment (LC_ALL=C, locale-coercion + UTF-8 mode disabled, piped stdout): pre-fix reproduces the byte-for-byte position 80 crash; post-fix flips the codec to UTF-8 and exits 0.
  • Full binary/tests setup suite: 16/16 pass (the 4 test_hook_cli failures are pre-existing on the base — they need a built discovery binary not present in the checkout).

Note

The bash onboard.sh.tmpl already POSTs install failures to /api/v1/mdm/install-report, but that backend endpoint does not exist yet, so these failures are currently invisible server-side. Tracked separately.

Greptile Summary

This PR fixes a UnicodeEncodeError crash in unbound-hook setup that aborted onboarding on Jamf fleet check-ins due to Python falling back to the ASCII stdout codec in a launchd context with no LANG/LC_* set.

  • main.py: Adds _force_utf8_io(), called at the top of main(), which uses TextIOWrapper.reconfigure() to switch stdout/stderr to UTF-8 with errors='replace' so no print can raise UnicodeEncodeError.
  • setup_cmd.py: Replaces the (U+2192) migration banner arrow with ASCII -> as defense in depth.
  • test_setup_migration.py: Adds test_setup_survives_ascii_stdout, which reproduces the exact crash scenario (ASCII-backed TextIOWrapper piped to sys.stdout) and verifies it exits 0 post-fix.

Confidence Score: 5/5

Safe to merge — the change is a narrow, well-contained fix for a crash-on-launch bug with no behavioral changes to any non-output code path.

The fix touches only stream reconfiguration at process startup and a one-character banner swap. The core setup logic (settings writes, discovery, key fetch) is untouched. The new regression test accurately reproduces the field crash end-to-end and passes. The reconfigure() call is guarded against both a missing method and runtime exceptions, so the fix cannot itself become a new crash vector.

No files require special attention.

Important Files Changed

Filename Overview
binary/src/unbound_hook/main.py Adds _force_utf8_io() called at main() entry; correctly guards against missing reconfigure() and handles ValueError/OSError. Previously-flagged concerns (errors='replace' on the hook JSON path, silent pass) are covered in existing review threads.
binary/src/unbound_hook/setup_cmd.py One-character change: U+2192 arrow swapped for ASCII '->' in the migration banner — correct defense-in-depth fix.
binary/tests/test_setup_migration.py New test faithfully reproduces the field crash (ASCII TextIOWrapper on sys.stdout/stderr), exercises main() end-to-end, and asserts both codec reconfiguration and that the banner reached the output buffer.

Sequence Diagram

sequenceDiagram
    participant Jamf as Jamf launchd (no LANG/LC_*)
    participant main as main()
    participant utf8 as _force_utf8_io()
    participant stdout as sys.stdout
    participant setup as setup_cmd.run()

    Jamf->>main: invoke unbound-hook setup --api-key ...
    main->>utf8: _force_utf8_io()
    utf8->>stdout: "reconfigure(encoding="utf-8", errors="replace")"
    Note over stdout: codec: ASCII → UTF-8 (errors=replace)
    utf8-->>main: done (or silently skips on ValueError/OSError)
    main->>setup: setup_cmd.run(rest)
    setup->>stdout: "print("[migration] python->binary sweep")"
    Note over stdout: ASCII arrow, no UnicodeEncodeError
    setup-->>main: "rc=0"
    main-->>Jamf: exit 0
Loading

Reviews (2): Last reviewed commit: "fix(unbound-hook): never let stdout enco..." | Re-trigger Greptile

if reconfigure is None:
continue
try:
reconfigure(encoding="utf-8", errors="replace")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 errors='replace' applied to the hook subcommand's stdout silently replaces unencodable characters (surrogates) with ?. For the diagnostic setup path this is intentional and correct; but the hook path writes structured JSON consumed by Claude Code's hook dispatcher. If a surrogate ever appears in the output, the JSON would be silently corrupted and the hook's allow/block decision would be lost without any indication. Consider using errors='surrogateescape' instead — it round-trips the raw bytes faithfully rather than dropping characters, so the JSON consumer either parses it correctly or fails with a clear decode error.

Comment thread binary/src/unbound_hook/main.py
Jamf's recurring check-in runs the MDM onboarding policy from a launchd
context with no LANG/LC_* set, so the interpreter falls back to the ASCII
stdout codec. The first diagnostic line setup prints — the migration
banner "[migration] python->binary sweep" — contained a U+2192 arrow,
which raised UnicodeEncodeError and aborted `setup` (exit 1 ->
UNBOUND_INSTALL_FAILED step=setup) on every check-in. Interactive
`sudo jamf policy` runs inherit a UTF-8 locale and passed, which masked
it during canary. The output is purely diagnostic; a cosmetic write
should never kill the install.

- main(): reconfigure stdout/stderr to UTF-8 (errors='replace') before
  dispatching any subcommand, so no print can raise UnicodeEncodeError.
  Also hardens the `hook` path, whose stdout carries UTF-8 JSON.
- setup_cmd: swap the banner arrow for ASCII '->' as defense in depth.
- test: drive main(["setup", ...]) with ASCII-backed streams; reproduces
  the exact crash pre-fix, passes post-fix.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@vigneshsubbiah16 vigneshsubbiah16 force-pushed the fix/unbound-hook-stdout-encoding branch from d889743 to 209a03a Compare June 15, 2026 17:48
@vigneshsubbiah16 vigneshsubbiah16 changed the base branch from feat/unbound-hook-binary to staging June 15, 2026 17:48
@vigneshsubbiah16 vigneshsubbiah16 requested a review from a team June 15, 2026 17:48
@vigneshsubbiah16 vigneshsubbiah16 merged commit 4f37992 into staging Jun 15, 2026
2 checks passed
vigneshsubbiah16 added a commit that referenced this pull request Jun 15, 2026
Lockstep bump of both binaries' __version__ (unbound-hook + discovery) so
the release workflow's publish-safety assert passes when tagging
runtime-v0.1.5. This is the release that carries the stdout-encoding fix
(#153) to the fleet. No behavior change.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
vigneshsubbiah16 added a commit that referenced this pull request Jun 15, 2026
* fix(unbound-hook): never let stdout encoding abort onboarding setup (#153)

Jamf's recurring check-in runs the MDM onboarding policy from a launchd
context with no LANG/LC_* set, so the interpreter falls back to the ASCII
stdout codec. The first diagnostic line setup prints — the migration
banner "[migration] python->binary sweep" — contained a U+2192 arrow,
which raised UnicodeEncodeError and aborted `setup` (exit 1 ->
UNBOUND_INSTALL_FAILED step=setup) on every check-in. Interactive
`sudo jamf policy` runs inherit a UTF-8 locale and passed, which masked
it during canary. The output is purely diagnostic; a cosmetic write
should never kill the install.

- main(): reconfigure stdout/stderr to UTF-8 (errors='replace') before
  dispatching any subcommand, so no print can raise UnicodeEncodeError.
  Also hardens the `hook` path, whose stdout carries UTF-8 JSON.
- setup_cmd: swap the banner arrow for ASCII '->' as defense in depth.
- test: drive main(["setup", ...]) with ASCII-backed streams; reproduces
  the exact crash pre-fix, passes post-fix.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* chore(release): bump runtime version 0.1.4 -> 0.1.5 (#154)

Lockstep bump of both binaries' __version__ (unbound-hook + discovery) so
the release workflow's publish-safety assert passes when tagging
runtime-v0.1.5. This is the release that carries the stdout-encoding fix
(#153) to the fleet. No behavior change.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

1 participant