The wrapper reconciles whichever Review the service returns without checking it is for this repository and branch #53

New issue

Open

opened 2026-10-06 05:35:52 +00:00 by jercik · 0 comments

jercik commented

2026-10-06 05:35:52 +00:00

Owner

The wrapper never compares the Review it gets back with the one it asked for. A Review of another repository or target branch would be posted to the pull request.

What happens

After the create call, review() takes the returned Review's id and goes on to reconcile it (src/wrapper/orchestrator.ts:84-92). The bundled client checks that the response has the right shape, and a malformed Review ends the run before any write. Beyond that, the only comparison is the Review's derivation.subject_commit against the head, and it only decides whether to log a line (src/wrapper/orchestrator.ts:93-98). Repository, scope kind, target branch and tree are never compared. No mismatch stops the run.

Reproduced at 9f272a52 with the committed bundle on Node 24.21.0 against a stand-in service. For a request about acme/widgets, a Review of other/elsewhere on another forge host, with target release/9 and commit 9999…, was reconciled on an event run (s5a-foreign-prt) and on a dispatch run (s5a-foreign-dispatch). Each wrote the summary comment and an inline review to the pull request.

What it should do instead

Compare the Review's repo.forge_host, repo.slug, scope_kind and derivation.target_ref with what the wrapper asked for (inputs.forgeHost, inputs.slug, a diff scope, inputs.baseBranch), and end the run before any write when one differs. A different subject_commit stays acceptable: an event run sends attach_same_diff: true (src/wrapper/orchestrator.ts:90, added in #29), so the service may attach an earlier commit's Review.

Why it matters

The service decides which Review a pull request receives, and the wrapper posts it under that pull request's name. A service bug or a compromised service could post another repository's findings, which may quote that repository's code, onto this pull request.

The service is a trusted party, pinned by expected-service-origin. This is a guard against service faults, not against pull request authors, and the comparison is four fields.

🤖 Generated with Claude Code

The wrapper never compares the Review it gets back with the one it asked for. A Review of another repository or target branch would be posted to the pull request. ## What happens After the create call, `review()` takes the returned Review's id and goes on to reconcile it ([`src/wrapper/orchestrator.ts:84-92`](https://code.j4k.dev/j4k-oss/review-wrapper/src/commit/9f272a522e8ed6f748533761de45ee3e94fd7481/src/wrapper/orchestrator.ts#L84-L92)). The bundled client checks that the response has the right shape, and a malformed Review ends the run before any write. Beyond that, the only comparison is the Review's `derivation.subject_commit` against the head, and it only decides whether to log a line ([`src/wrapper/orchestrator.ts:93-98`](https://code.j4k.dev/j4k-oss/review-wrapper/src/commit/9f272a522e8ed6f748533761de45ee3e94fd7481/src/wrapper/orchestrator.ts#L93-L98)). Repository, scope kind, target branch and tree are never compared. No mismatch stops the run. Reproduced at `9f272a52` with the committed bundle on Node 24.21.0 against a stand-in service. For a request about `acme/widgets`, a Review of `other/elsewhere` on another forge host, with target `release/9` and commit `9999…`, was reconciled on an event run (`s5a-foreign-prt`) and on a dispatch run (`s5a-foreign-dispatch`). Each wrote the summary comment and an inline review to the pull request. ## What it should do instead Compare the Review's `repo.forge_host`, `repo.slug`, `scope_kind` and `derivation.target_ref` with what the wrapper asked for (`inputs.forgeHost`, `inputs.slug`, a `diff` scope, `inputs.baseBranch`), and end the run before any write when one differs. A different `subject_commit` stays acceptable: an event run sends `attach_same_diff: true` ([`src/wrapper/orchestrator.ts:90`](https://code.j4k.dev/j4k-oss/review-wrapper/src/commit/9f272a522e8ed6f748533761de45ee3e94fd7481/src/wrapper/orchestrator.ts#L90), added in #29), so the service may attach an earlier commit's Review. ## Why it matters The service decides which Review a pull request receives, and the wrapper posts it under that pull request's name. A service bug or a compromised service could post another repository's findings, which may quote that repository's code, onto this pull request. The service is a trusted party, pinned by `expected-service-origin`. This is a guard against service faults, not against pull request authors, and the comparison is four fields. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

No labels

No milestone

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

j4k-oss/review-wrapper#53

No description provided.

Rows
Columns