luigidemasi opened a new issue, #80: URL: https://github.com/apache/camel-kit/issues/80
> Imported from [luigidemasi/camel-kit#169](https://github.com/luigidemasi/camel-kit/issues/169), opened by [luigidemasi](https://github.com/luigidemasi) on 2026-07-30. > Closed by [luigidemasi/camel-kit#196](https://github.com/luigidemasi/camel-kit/pull/196), merged on 2026-08-31. --- ## Context Applies to the Ship workflow coordinator introduced in luigidemasi/camel-kit#168 (code references at commit `889c5c70` on `issue-149/13-coordinator`). This was noted as a pre-existing, non-blocking observation in the fix-verification comment on that PR; it exists unchanged at the PR's initial commit and is not a regression of the review fixes. ## Problem A durable Pi stage result marker that has been edited on disk to carry an out-of-bounds `warning` field makes the VALIDATE stage fail with an uncaught `IllegalArgumentException` instead of the typed tampered-evidence rejection used everywhere else on the recovery path. Chain: 1. `ShipCoordinator.validate()` builds the stamp `ToolVersion` from the recovered EXECUTE result, including `executeResult.warning()` (`ShipCoordinator.java:455-460`). 2. The marker's `warning` field is not covered by any integrity pin: `Marker.matches(request)` checks run/stage/attempt/input digest only, the log digests cover stdout/stderr contents, and the `assistantText` output-digest comparison in `completedPiResult` is skipped for EXECUTE (`ShipCoordinator.java:694-696`). An edited `warning` therefore survives `PiWorkerResultStore.read` verification. 3. `ShipLocalStamp.ToolVersion` routes the warning through `requireText`, which throws `IllegalArgumentException` for blank, longer than 4096 chars, or unsafe display characters (`ShipLocalStamp.java:160`, `342-348`). 4. `validate()` catches only `SessionBusyException` and `IOException` (`ShipCoordinator.java:472-481`), so the `IllegalArgumentException` escapes through `run()`/`resume()` uncaught. The VALIDATE stage stays RUNNING at the same attempt and every retry fails identically until the marker edit is reverted or local state is reset — the wedge that luigidemasi/camel-kit#168 eliminated for I/O failures survives in this corridor. ## Severity assessment Low. The worker cannot produce such a warning: live warnings are composed from fixed literals plus a version bounded to 1024 chars and matched against a strict pattern (`PiWorker.java:80`, `97`, `1363`), so only direct same-user modification of a `0600` marker file reaches the throw. The behavior is fail-closed (nothing is persisted, no attempt is consumed) and self-reversible. This is a robustness/consistency gap, not an exploitable one. ## Suggested hardening Enforce bounds on the recovered `Result` text fields (`warning`, and any other free-text field that reaches `ShipLocalStamp`) at the recovery trust boundary — `PiWorkerResultStore.read`/`verifyEvidence` — throwing the existing `PiWorker.UntrustedResultException`, so an out-of-bounds field is classified as tampered evidence like every other marker corruption (typed, deterministic, restart/fail semantics decided by the caller) instead of escaping as an unchecked exception from stamp construction. A regression test would mirror the existing tampered-marker tests in `PiWorkerResultStoreTest` with an oversized `warning`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
