Skip to content

Upstream Mike PR #334 review - #135

Draft
github-actions[bot] wants to merge 1 commit into
mainfrom
agent/upstream-sync-high-20260823155921
Draft

Upstream Mike PR #334 review#135
github-actions[bot] wants to merge 1 commit into
mainfrom
agent/upstream-sync-high-20260823155921

Conversation

@github-actions

Copy link
Copy Markdown

Automated-Upstream-Mike-Sync: true
Upstream-Risk: high

Promising capability, but the combined change crosses authentication, MFA, service-role secrets, and admin-client lifecycle boundaries. Security review and ROSS-native tests are required before implementation.

Mike PR open-legal-products#334

  • Outcome: needs-decision
  • Capability: Sanitized storage-failure logging plus centralized reuse of the server Supabase admin client in authentication and MFA middleware
  • Series: none
  • Dependencies: backend/src/lib/safeError.ts, backend/src/lib/storage.ts, backend/src/lib/supabase.ts, backend/src/middleware/auth.ts, @supabase/supabase-js, AWS S3 SDK
  • Prerequisites: Security approval for caching a service-role client across requests, ROSS-native node:test coverage for auth, MFA, configuration changes, and sanitized storage errors
  • Reason: ROSS has matching seams and safeErrorLog already exists, but the upstream tests use unavailable Vitest and do not establish that shared service-role client reuse preserves token isolation, MFA enforcement, configuration rotation, and secret redaction.

Architecture brief

Add sanitized failure logging only at storage operation boundaries. If approved, keep the service-role client cache private to backend/src/lib/supabase.ts, disable session persistence and token refresh, invalidate it when URL or key changes, and make auth/MFA middleware obtain the client exclusively through that factory without changing response semantics.

Implementation plan

  • Create ROSS node:test harnesses for download and signed-URL failures, asserting redaction and object-key handling.
  • Test factory reuse, configuration validation and rotation, disabled session persistence and refresh, and isolation across sequential requests with different bearer tokens.
  • Exercise requireAuth and requireMfaIfEnrolled for missing configuration, invalid tokens, verified-email enforcement, AAL1/AAL2 transitions, and MFA bootstrap routes.
  • Obtain security review of service-role client lifetime and logged object-key sensitivity, then implement the storage and auth portions as separately reviewable changes.

Required human action

This is a draft state-only architecture record. Review the brief and implementation plan before any code is attempted.

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.

0 participants