Skip to content

fix(security): harden privilege boundaries, ws auth and outbox ordering - #39

Merged
bbbbbbbbbbbbba merged 1 commit into
huabeitech:mainfrom
DOS:fix/upstream-security-hardening
Sep 9, 2026
Merged

bbbbbbbbbbbbba merged 1 commit into
huabeitech:mainfrom
DOS:fix/upstream-security-hardening

Conversation

@JOY

@JOY JOY commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

What

Security hardening for privilege boundaries, realtime authorization, webhook config and the outbound delivery queue. Contains no dependency or feature changes — 5 files, +59/−22.

Fixes

ID Fix
SEC-02 (critical) ResetPassword / password change: an operator who is not a super_admin can no longer reset the password of a user that holds a super_admin role — previously a single admin request returned the super_admin plaintext password
SEC-03 (critical) AssignRoles: system (IsSystem) roles can no longer be granted unless the operator is a super_admin — previously an admin could self-assign super_admin
SEC-04 (critical) AssignPermissions: refuses to rewrite the permission set of built-in IsSystem roles (e.g. super_admin) unless the operator is a super_admin — matching the guard DeleteRole already had
SEC-07 (high) canSubscribeConversation: staff websocket sessions now require the same conversation.view permission the REST conversation endpoints gate with — previously any authenticated employee could subscribe to conversation:<id> topics and read full message content
BUG-04 (high) ListPending only returns outbox rows whose next_retry_at backoff has elapsed, ordered by next_retry_at — a backlog of not-yet-due retries no longer starves newer pending sends every 5 s
SEC-14 (medium) bindEnvironmentAliases binds the documented AGENT_DESK_* aliases before the legacy names, so ambient variables like PORT / DATABASE_URL cannot silently override the YAML/prefixed configuration

Verification

  • go build -tags dev ./internal/... — pass
  • go vet — pass (no new findings)
  • go test -count=1 -tags dev ./internal/services/... ./internal/repositories/... ./internal/pkg/... — all packages pass except the two pre-existing internal/pkg/config tests that fail on machines exporting ambient PORT/DATABASE_URL (documented environment contamination — they pass on clean CI runners; the bindEnvironmentAliases reordering in this PR is the documented-contract fix for the alias-precedence issue, and the remaining hermetic-test gap is tracked separately)

Known follow-ups (not in this PR)

  • SEC-01 (critical): file-upload allowlist + content sniffing (needs serving-header changes too)
  • SEC-05 (critical): guest identity binding at session_exchange (design change: server-issued anonymous identity)
  • SEC-10: hash session tokens at rest (needs a migration so existing sessions keep working)
  • SEC-12: enforce timestamped webhook signatures (compat decision per sender)
  • BUG-01 / BUG-17-19: i18n message-file repairs (mechanical JSON work, separate PR)
  • PERF-01: batch embedding API calls

…and env aliasing

- user_service: ResetPassword and AssignRoles can no longer touch super_admin targets or grant IsSystem roles unless the operator is a super_admin (was: any admin could escalate to super_admin in one request)

- role_service: AssignPermissions refuses to rewrite built-in IsSystem roles (AssignRoles guard) unless the operator is a super_admin

- ws_service: canSubscribeConversation now requires the same conversation.view permission the REST conversation endpoints gate with, closing a websocket read bypass of full message content

- channel_message_outbox: ListPending only returns rows whose backoff elapsed (next_retry_at) ordered by next_retry_at, so a backlog of not-yet-due retries no longer starves newer pending sends

- config: AGENT_DESK_* prefixed aliases are bound first so ambient legacy variables (PORT, DATABASE_URL) cannot silently override the documented configuration
@bbbbbbbbbbbbba
bbbbbbbbbbbbba merged commit 9dbb15f into huabeitech:main Sep 9, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants