fix(security): close unauth info-disclosure on setup-status, pat-status, app-state, /health#44
Open
dgokeeffe wants to merge 1 commit into
Open
fix(security): close unauth info-disclosure on setup-status, pat-status, app-state, /health#44dgokeeffe wants to merge 1 commit into
dgokeeffe wants to merge 1 commit into
Conversation
…t-status, app-state, /health) Closes the three P3/P4 info-disclosure findings from the 2026-05-17 pen test: 1. /api/setup-status was auth-exempt and returned the full setup_state dict including step error messages (stderr trimmed to 500 chars). If a setup script failed with leaky stderr — paths, version strings, env-var echoes — anyone hitting the URL could read it. 2. /api/pat-status was auth-exempt and returned workspace_host. Already known to anyone who reached the URL (they're auth'd to the same workspace), but principle-of-least-info says trim it. 3. /api/app-state was auth-exempt and returned app_owner email + last PAT rotation timestamp. PR #40 also removes this — including the same one-line fix here lets it ship sooner; PR #40 will rebase cleanly. 4. /health was auth-exempt and returned version, active_sessions, setup_status, session_timeout_seconds. Version exposure enables CVE-targeted exploit selection. Trimmed to {"status": "healthy"} — no internal or external caller needs the rich shape (the frontend doesn't poll /health; the Apps platform uses its own liveness mechanism upstream of the app). Removed setup-status, pat-status, and app-state from the before_request auth-exempt list. The frontend continues to poll all three because it loads from / (auth'd), so it already has SSO cookies. Test changes: - 5 new tests in TestInfoDisclosureEndpoints covering: setup-status denied/allowed, pat-status denied, app-state denied, /health minimal-response anti-leak (explicit assertions that version, setup_status, active_sessions are NOT in the response). - Updated test_session_linger.py: the 24h timeout assertion now reads SESSION_TIMEOUT_SECONDS from the module directly instead of via /health, since exposing the value to unauth callers was the leak we're closing. Stronger test, smaller attack surface. 236/236 unit tests pass. Co-authored-by: Isaac
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the three P3/P4 info-disclosure findings from the 2026-05-17 pen test, bundled because each is a single-line/single-block change in the same file. Companion to the merged hotfix #42 (configure-pat owner-gate).
The findings (from
.humantokens/2026-05-17-pentest-findings.md)/api/setup-status/api/pat-status/api/app-state/healthWhy these changes are safe
Tests
What this PR is NOT
Test plan
This pull request and its description were written by Isaac.