Forge-controlled text reaches the console unsanitized through the gh and glab adapters #469

New issue

mph · 2026-08-27T11:22:48Z

mph commented

2026-08-27 11:22:48 +00:00

Found triaging IW-437 (noticed during phase 2 code review, while hardening the Forgejo HTTP path).

WHAT. GitHubClient.createPullRequest (core/adapters/GitHubClient.scala:688-712) and GitLabClient.createMergeRequest (core/adapters/GitLabClient.scala:568-592) pass the raw stderr of gh and glab into ForgeCreateError.Other with no filtering and no length limit. That message travels through PhasePr.createPrAndGate (core/commands/PhasePr.scala:121-125) to System.err.println in LiveConsole.err (core/commands/LiveCommandEnv.scala:61).

THE ASYMMETRY. ForgejoClient.bodySnippet strips control characters and bidirectional formatting characters and caps the length of an HTTP response body. The two CLI adapters apply no equivalent to the subprocess stderr they forward. So a remote whose branch name, title or error text carries an ANSI escape or a right-to-left override can still reorder or repaint what iw phase-pr prints - through the CLI path rather than the HTTP path.

THE GENERAL POINT. Text that a forge controls reaches the console through EVERY transport an adapter uses, so a sanitizer placed on one transport leaves the others open.

PROPOSED FIX. One shared sanitizer applied to every forge-controlled string an adapter forwards to the console: the gh and glab stderr paths as well as the Forgejo response body. Extracting the existing bodySnippet logic is the obvious starting point.

PRE-EXISTING. Nothing in IW-437 introduced the CLI path or changed how it forwards stderr; phase 2 was scoped to the Forgejo create-pull-request fall-through.

Found triaging IW-437 (noticed during phase 2 code review, while hardening the Forgejo HTTP path). WHAT. GitHubClient.createPullRequest (core/adapters/GitHubClient.scala:688-712) and GitLabClient.createMergeRequest (core/adapters/GitLabClient.scala:568-592) pass the raw stderr of gh and glab into ForgeCreateError.Other with no filtering and no length limit. That message travels through PhasePr.createPrAndGate (core/commands/PhasePr.scala:121-125) to System.err.println in LiveConsole.err (core/commands/LiveCommandEnv.scala:61). THE ASYMMETRY. ForgejoClient.bodySnippet strips control characters and bidirectional formatting characters and caps the length of an HTTP response body. The two CLI adapters apply no equivalent to the subprocess stderr they forward. So a remote whose branch name, title or error text carries an ANSI escape or a right-to-left override can still reorder or repaint what iw phase-pr prints - through the CLI path rather than the HTTP path. THE GENERAL POINT. Text that a forge controls reaches the console through EVERY transport an adapter uses, so a sanitizer placed on one transport leaves the others open. PROPOSED FIX. One shared sanitizer applied to every forge-controlled string an adapter forwards to the console: the gh and glab stderr paths as well as the Forgejo response body. Extracting the existing bodySnippet logic is the obvious starting point. PRE-EXISTING. Nothing in IW-437 introduced the CLI path or changed how it forwards stderr; phase 2 was scoped to the Forgejo create-pull-request fall-through.

mph referenced this issue

2026-08-28 07:22:08 +00:00

IW-437: The phase CI gate finishes a phase without manual intervention #475

claude commented

2026-10-15 23:29:58 +00:00

Intake: agent-ready
Workflow: diagnostic
Reason: all gates passed
Analysis: project-management/issues/IW-469/analysis.md on branch IW-469

Intake: agent-ready Workflow: diagnostic Reason: all gates passed Analysis: project-management/issues/IW-469/analysis.md on branch IW-469

claude added the

agent-ready

label

2026-10-15 23:29:58 +00:00

claude commented

2026-10-15 23:38:22 +00:00

Implement: needs-human
Reason: batch-implement exit 1 — the kanon plugin did not compile against the iw-cli core of this branch, so no phase ran. fetchPullRequestForBranch now returns Either[TrackerError, …] (iw-cli #778), and the installed kanon plugin (lib/batch/ForgeCatchUp.scala) still expects Either[String, …]. The issue itself is not at fault: update the kanon plugin, then release the issue to agent-ready again. batch-implement.log was not written; the lines below are the command output.

[error] /home/mph/.local/share/iw/plugins/kanon/lib/batch/ForgeCatchUp.scala:143:7
[error] Found:    Either[iw.core.model.TrackerError, Option[batch.ForgeState]]
[error] Required: Either[String, Option[batch.ForgeState]]
Compilation failed

Implement: needs-human Reason: batch-implement exit 1 — the kanon plugin did not compile against the iw-cli core of this branch, so no phase ran. `fetchPullRequestForBranch` now returns `Either[TrackerError, …]` (iw-cli #778), and the installed kanon plugin (`lib/batch/ForgeCatchUp.scala`) still expects `Either[String, …]`. The issue itself is not at fault: update the kanon plugin, then release the issue to `agent-ready` again. batch-implement.log was not written; the lines below are the command output. ``` [error] /home/mph/.local/share/iw/plugins/kanon/lib/batch/ForgeCatchUp.scala:143:7 [error] Found: Either[iw.core.model.TrackerError, Option[batch.ForgeState]] [error] Required: Either[String, Option[batch.ForgeState]] Compilation failed ```

claude added

needs-human

and removed

agent-ready

labels

2026-10-15 23:38:22 +00:00

claude commented

2026-10-16 17:10:33 +00:00

Release: the implement loop failed this issue for a reason outside it. The kanon plugin did not compile against iw-cli main, because the tracker port now returns TrackerError (iw-cli #778). The plugin fix is dev-docs #484, and iw-cli 0.22.1 carries the core change. The issue goes back to the implement loop.

claude added

agent-ready

and removed

needs-human

labels

2026-10-16 17:10:34 +00:00

Sign in to join this conversation.