fix(worker): hide internal session control failures - #121
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
P2
|
Codex review: needs maintainer review before merge. Reviewed August 29, 2026, 2:31 AM ET / 06:31 UTC. ClawSweeper reviewWhat this changesThe PR replaces exception text in Session Control HTTP 500 responses with a fixed Merge readinessThe PR is still necessary: current main returns caught exception text from this Durable Object, while this branch consistently returns a generic 500 response. The introduced change is correct and narrow; as a MEMBER-authored PR it should receive normal maintainer approval after required checks complete. Priority: P2 Review scores
Verification
How this fits togetherSession Control is a Worker Durable Object that receives internal binding requests for sandbox policies, checkpoints, and GitHub Actions relay state. It converts those operations into JSON HTTP responses consumed by Worker services. flowchart LR
A[Worker service] --> B[Session Control binding]
B --> C[Session Control Durable Object]
C --> D[Policy, checkpoint, or relay operation]
D --> E{Operation outcome}
E -->|success| F[JSON response]
E -->|unexpected failure| G[Generic 500 response]
F --> H[Worker caller]
G --> H
Decision needed
Why: Repository policy keeps MEMBER-authored items out of automatic closure, so merge authority must make the final landing decision despite the narrow correct patch. Before merge
Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Keep the fixed generic 500 response at the Session Control boundary so unexpected internal failures retain their status without disclosing exception details. Do we have a high-confidence way to reproduce the issue? Yes, source-reproducible: malformed JSON or another thrown operation reaches this Is this the best way to solve the issue? Yes. Replacing the catch response at the sole Durable Object boundary is the narrowest maintainable way to preserve HTTP 500 semantics while preventing exception-detail disclosure. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 4e10690d2cf8. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (1 earlier review cycle)
|
1 participant
What Problem This Solves
Fixes an issue where internal Session Control failures could return parser or exception details in an HTTP 500 response.
Related: https://github.com/openclaw/crabfleet/security/code-scanning/4
Why This Change Was Made
The Session Control Durable Object now owns one fixed internal-error response for unexpected failures. Caught values are no longer serialized across the response boundary.
User Impact
Unexpected Session Control failures remain visible as HTTP 500 responses without exposing internal error details.
Evidence
{"error":"internal error"}with no synthetic or parser detail.pnpm checkpnpm test(1,003 passed)pnpm buildpnpm exec wrangler deploy --dry-run --containers-rollout=noneDEVELOPER_DIR=/Applications/Xcode.app/Contents/Developer pnpm macos:testgpt-5.6-solhigh review: no findings.Production LOC: +2/-5 (net -3). Tests: +0/-0.