🛡️ Sentinel: [MEDIUM] Fix unhandled json.Marshal errors#95
Conversation
**Severity:** MEDIUM **Vulnerability:** Calls to `json.Marshal` in API handlers (`internal/api/handlers.go`) ignored returned errors using the blank identifier `_`. If marshaling failed, this resulted in an invalid, empty, or incorrectly processed payload without triggering an immediate, visible error. **Impact:** Silently enqueueing corrupted or empty job payloads which could cause downstream job execution failures or subtle state corruption. **Fix:** Explicitly handle errors returned by `json.Marshal`. If an error occurs, it is logged with `s.logger.Error` and an `http.StatusInternalServerError` (500) is returned to the caller. **Verification:** Reviewed code changes and ran the `internal/api` test suite which passed successfully. Co-authored-by: mattjoyce <278869+mattjoyce@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
- #96 (ADR vocab sync) -> done. - #83 epic: luminary code review done (unanimous approve, zero blockers); Tier A+B folded; T7 finding (live host loads a vault -> compose-attestation active, but no accounts map -> privsep unconfined, enforce macOS-pending #95). Next: doc review. - #97: deferred non-blocking review follow-ups (T3, T5, T9, T15, vocab lint).
ADR (explanation): T5c downgrade-vs-secret-denial asymmetry in §4; T17 point-in-time-at-boot tradeoff in §8. DEPLOYMENT.md: §5b how-to gains T11 uid-coupling SSOT, T12 Linux-proven/ macOS-pending (#95), T13 root-refusal dev note, and account:→run_as: drift fix; new §5c reference (keys, reserved keywords, boot-gate matrix, failure modes). Schema: add service.unconfined to ServiceConfig — was a live key absent from the strict schema, so config validate --whole would reject the one documented escape hatch. Closes #98.
1 participant
This PR addresses a medium-severity security issue where errors from
json.Marshalininternal/api/handlers.gowere being ignored. If a payload failed to serialize, the application would silently proceed, potentially enqueueing corrupted data.The fix explicitly handles these errors by logging them and returning an HTTP 500 Internal Server Error, adhering to secure failure principles.
A sentinel journal entry (
.jules/sentinel.md) was also added to document this learning.PR created automatically by Jules for task 16394824879684129011 started by @mattjoyce