Skip to content

Follow-ups from #912: auto-commit refusal hardening and HOME resolution #917

Open

Description

@tps-flint

Follow-ups from the review of #912 (non-blocking), tracked here as one issue:

Auto-commit refusal (runAutoCommit, codex-runtime.ts)

  1. taskId in the blocker summary is not sanitized. The mail path validates message ids, but the Flair task loop takes event.refId ?? event.id, and refId is only type-checked. Validate or sanitize it before it reaches the summary or the error.
  2. The repository path and a spawn error's message enter the error text unsanitized; only the probe's stderr is sanitized.
  3. If publishing the blocker fails, the refusal again ends as a warning and a null PR URL in _runAutoCommitLegacy, after the task was reported complete. Consider failing the task itself.
  4. publishEvent has no timeout; a hang blocks the refusal (it still fails closed).
  5. The sanitizer leaves non-Cc display controls (U+2028/U+2029, bidi overrides U+202A-U+202E and U+2066-U+2069), and .slice(0, 300) can leave a lone surrogate. Decide whether those belong in the replaced set.
  6. Text: the function comment omits the .trim(), and "Remove ANSI CSI escapes" does not cover the 8-bit CSI (U+009B is replaced by a space and its parameter bytes stay as text).
  7. The long-stderr test pins the cap only to about 456 characters; an assertion on the exact length would pin 300.
  8. No real-Flair test covers the blocker event (the test uses a fake publisher).

HOME resolution (#822)
9. About 30 other files under packages/cli/src still resolve paths with os.homedir(). None is reachable from the runtimes changed here, and none is wrong while nothing changes HOME in-process, but consolidating on homeDir() removes the trap that produced #822.
10. bin/tps.ts builds the runtime's mailDir/extraDirs with homedir() at launch; if in-process HOME changes are supported, those stay on the start-up HOME.
11. The Gemini runtime prepends <HOME>/.tps/bin to the child's PATH, so spawn("gemini") also resolves through it; with a per-task, mutable HOME, resolve gemini before prepending.
12. flair-client.ts defaultFlairKeyPath() still uses homedir() (unreachable from these runtimes, which require flairKeyPath).

Refs #822, #824, #912.

Activity

  1. moved this to Triage in Office queueon Nov 10, 2026
  2. tps-flint commented on Nov 10, 2026

    @tps-flint
    ContributorAuthor

    On item 1, the shape Sherlock recommended after #912's review: constrain refId once at ingest rather than sanitizing each call site. org-event-read.ts rejects a refId over a bounded length (e.g. 128) or outside a safe character class (no C0/C1 controls, no U+2028/U+2029), with the matching constraint on the Flair OrgEvent schema. That one change covers every summary that interpolates taskId (the refusal blocker, Completed:, PR opened for, Task … failed, both PR creation failed for sites), not only the one #912 touched.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions