Skip to content

chore: move multi error handler to shared#2333

Draft
jakubno wants to merge 1 commit intomainfrom
chore/move-multi-error-handler-to-shared
Draft

chore: move multi error handler to shared#2333
jakubno wants to merge 1 commit intomainfrom
chore/move-multi-error-handler-to-shared

Conversation

@jakubno
Copy link
Copy Markdown
Member

@jakubno jakubno commented Apr 8, 2026

No description provided.

@cursor
Copy link
Copy Markdown

cursor bot commented Apr 8, 2026

PR Summary

Medium Risk
Touches request validation/auth error handling paths, so small mismatches in prefixes or error types could change client-visible status codes/messages for forbidden/blocked/auth failures. Behavior is largely a refactor but affects multiple services’ middleware.

Overview
Moves OpenAPI request MultiErrorHandler logic (including SecurityRequirementsError unwrapping and team forbidden/blocked classification) out of the API service into packages/auth/pkg/auth, and updates both api and dashboard-api to use the shared handler. The API’s ErrorHandler now relies on shared auth error-prefix constants, and the old db re-exports for team auth errors are removed, consolidating these error types under auth.

Reviewed by Cursor Bugbot for commit f6e3a1f. Bugbot is set up for automated code reviews on this repo. Configure here.


return fmt.Errorf("%s", strings.Join(msgs, "; "))
},
MultiErrorHandler: sharedauth.MultiErrorHandler,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Behavioral regression: the previous handler joined all errors from openapi3.MultiError; this replacement only processes me[0] and returns errors prefixed with SecurityErrPrefix for security failures. Unlike packages/api, the dashboard-api error handler does not strip these prefixes. Auth/security validation failures would now expose the raw internal error string in API responses.


return fmt.Errorf("%s", strings.Join(msgs, "; "))
},
MultiErrorHandler: sharedauth.MultiErrorHandler,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Behavioral regression: the previous handler joined all errors from openapi3.MultiError; this replacement only processes me[0] and returns errors prefixed with SecurityErrPrefix for security failures.

Unlike packages/api, the dashboard-api error handler does not strip these prefixes (there is no equivalent to the api ErrorHandler that calls strings.CutPrefix on auth.SecurityErrPrefix). Auth/security validation failures would now expose the raw internal string "error in openapi3filter.SecurityRequirementsError: security requirements failed: ..." in API responses.

func processCustomErrors(e *openapi3filter.SecurityRequirementsError) error {
// Return only one security requirement error (there may be multiple securitySchemes)
unwrapped := e.Errors
err := unwrapped[0]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential panic: unwrapped[0] is accessed without a bounds check. This risk already existed in packages/api, but it is now also reachable from dashboard-api, which previously used a simple join handler that never hit this code path. If a SecurityRequirementsError is ever produced with an empty Errors slice this will panic.

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