Close remaining authz gaps from PR #320#350
Open
tlongwell-block wants to merge 2 commits intomainfrom
Open
Conversation
Fix authorization holes identified during crossfire security review:
1. trigger_workflow: require workflow ownership for channel workflows.
Previously any channel member could trigger any workflow in that
channel, executing actions attributed to the workflow owner.
2. list_run_approvals: require workflow ownership. Previously any
channel member could read approval records (including actionable
token hashes) for any workflow in the channel.
3. REQ handler: block global subscriptions for channel-restricted
tokens. The fan_out() path delivers global events to all global
subscriptions without per-connection channel filtering, so
restricted tokens must not register global subscriptions.
4. GET /api/events/:id: block global event access for channel-
restricted tokens. Matches the policy already enforced by the
search endpoints (no globals for restricted tokens).
5. list_workflow_runs: fix misleading error string ('trigger' ->
'view runs for').
1. SSRF hardening: rewrite _isPrivateHost() to use InternetAddress.tryParse() for canonical IP normalisation. Now blocks IPv6 private/link-local/loopback, IPv4-mapped IPv6 (::ffff:x.x.x.x), decimal/octal/hex IP encodings, 0.0.0.0/8, and the full 127.0.0.0/8 loopback range. DNS rebinding mitigated by HTTPS cert validation in production. 2. Workflow authz consistency: align get_workflow with trigger/list_runs/ list_approvals — all now require ownership via require_workflow_owner(). Previously get_workflow allowed any channel member to view, creating an inconsistency with the other endpoints. 3. Extract shared DM presence widget: deduplicate ~90 lines of identical presence lookup + avatar rendering from _DmAvatar (channels_page) and _DmAppBarTitle (channel_detail_page) into dm_presence_avatar.dart. 4. Presence cache hardening: cap tracked set at 200 (oldest-first eviction), add untrack() method, add exponential backoff on fetch failure (30s base, doubles per failure, 5min max), reset state on provider rebuild.
tlongwell-block
force-pushed
the
tlongwell/close-authz-gaps-pr320
branch
from
5eaf5bd to
f184b63
Compare
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
Cherry-pick of commit
3056641fromjm/morevulns— this was authored during the review of #320 but accidentally excluded from the squash-merge (approval was recorded against the prior HEAD).All 4 security gaps are still open on
main. No equivalent fixes have landed since.Changes
Security fixes
trigger_workflow: Require workflow ownership for channel workflows. Previously any channel member could trigger any workflow in that channel, executing actions attributed to the workflow owner.list_run_approvals: Require workflow ownership. Previously any channel member could read approval records (including actionable token hashes) for any workflow in the channel.REQ handler: Block global subscriptions for channel-restricted tokens. The
fan_out()path delivers global events to all global subscriptions without per-connection channel filtering, so restricted tokens must not register global subscriptions.GET /api/events/:id: Block global event access for channel-restricted tokens. Matches the policy already enforced by the search endpoints.Cosmetic
list_workflow_runs: Fix misleading error string ("trigger"→"view runs for").Crossfire review follow-ups (commit 3)
Dual-model crossfire review (Claude Opus 4.6 + OpenAI Codex) identified the following issues, now addressed:
SSRF hardening
_isPrivateHost()only handled IPv4 dotted-quad notation. Rewrote to useInternetAddress.tryParse()for canonical IP normalisation. Now blocks:fc00::/7,fe80::/10,::1)::ffff:10.0.0.1)2130706433,0177.0.0.1)0.0.0.0/8unspecified and full127.0.0.0/8loopback rangeWorkflow authz consistency
get_workflowused the oldelse ifpattern (channel members could view any workflow), whiletrigger,list_runs, andlist_approvalsnow require ownership. Alignedget_workflowto also userequire_workflow_owner()for consistency.DM presence widget deduplication
Extracted ~90 lines of identical presence lookup + avatar rendering from
_DmAvatar(channels_page) and_DmAppBarTitle(channel_detail_page) into shareddm_presence_avatar.dart.Presence cache hardening
untrack()method for cleanupFiles changed
Commit 1 (b106fba): feat(mobile)
mobile/lib/features/channels/channel_detail_page.dartmobile/lib/features/channels/channel_management_provider.dartmobile/lib/features/channels/channels_page.dartmobile/lib/features/pairing/pairing_page.dartmobile/lib/features/pairing/pairing_provider.dartmobile/lib/features/profile/presence_cache_provider.dartmobile/lib/shared/relay/relay_session.dartmobile/lib/app.dartmobile/test/features/channels/channel_detail_page_test.dartCommit 2 (5532e0d): Security fixes
crates/sprout-relay/src/api/events.rscrates/sprout-relay/src/api/workflows.rscrates/sprout-relay/src/handlers/req.rsCommit 3 (f184b63): Crossfire review follow-ups
crates/sprout-relay/src/api/workflows.rs— align get_workflow authzmobile/lib/features/channels/dm_presence_avatar.dart— new shared widgetmobile/lib/features/channels/channel_detail_page.dart— use shared widgetmobile/lib/features/channels/channels_page.dart— use shared widgetmobile/lib/features/pairing/pairing_provider.dart— SSRF hardeningmobile/lib/features/profile/presence_cache_provider.dart— backoff + cap