Return proper 4xx API responses on auth issues#285
Merged
inFocus7 merged 3 commits intoagentregistry-dev:mainfrom Mar 6, 2026
Merged
Return proper 4xx API responses on auth issues#285inFocus7 merged 3 commits intoagentregistry-dev:mainfrom
inFocus7 merged 3 commits intoagentregistry-dev:mainfrom
Conversation
Signed-off-by: Fabian Gonzalez <[email protected]>
Signed-off-by: Fabian Gonzalez <[email protected]>
Signed-off-by: Fabian Gonzalez <[email protected]>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR changes authentication/authorization error responses from the misleading HTTP 404 (Not Found) to proper HTTP 401 (Unauthorized) and 403 (Forbidden) responses across all API handler files. The motivation is improved UX: returning 404 when users attempt to modify resources they cannot access creates confusion, since users may incorrectly believe the resource doesn't exist rather than that they lack permissions.
Changes:
- Auth error mapping updated in all six handler files (
agents.go,deployments.go,edit.go,prompts.go,servers.go,skills.go):ErrUnauthenticatednow returns 401,ErrForbiddenreturns 403 - A new
removeDeploymentHTTPErrorhelper function is introduced indeployments.go, extracting and replacing inline error handling for deployment removal - Test expectations in
edit_test.goare updated from 404 to 401/403 as appropriate
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
internal/registry/api/handlers/v0/agents.go |
Auth errors now return 401/403 instead of 404 across all agent endpoints |
internal/registry/api/handlers/v0/deployments.go |
Auth errors return 401/403; new removeDeploymentHTTPError helper extracted |
internal/registry/api/handlers/v0/edit.go |
Auth errors now return 401/403 instead of 404 |
internal/registry/api/handlers/v0/prompts.go |
Auth errors now return 401/403 instead of 404 across all prompt endpoints |
internal/registry/api/handlers/v0/servers.go |
Auth errors now return 401/403; one block uses switch, others use if-chains |
internal/registry/api/handlers/v0/skills.go |
Auth errors now return 401/403 instead of 404 across all skill endpoints |
internal/registry/api/handlers/v0/edit_test.go |
Test expected statuses/errors updated to 401/403 to match new behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
peterj
approved these changes
Mar 6, 2026
christian-posta
pushed a commit
to christian-posta/agentregistry
that referenced
this pull request
Mar 9, 2026
<!-- Thanks for opening a PR! Please delete any sections that don't apply. --> # Description <!-- A concise explanation of the change. You may include: - **Motivation:** why this change is needed - **What changed:** key implementation details - **Related issues:** e.g., `Fixes agentregistry-dev#123` --> Originally I had preemptively decided on returning 404s instead of actual auth errors to prevent resource leakage. I'm walking back on this decision for a few reasons now: 1. This is very confusing UX when someone can view a resource, and when they try to modify it, they get 404s. - Similar to this, if I can view a resource now and in the future can't (due to authz), it would be confusing to see a 404 as I would believe the resource no longer exists, instead of the real issue: lack of permissions. Actually, that's just one reason, but still important. While the leakage prevention makes sense in public registries where any non-authenticated user can peruse, we currently do not have authn implemented that would warrant this precaution. If we ever truly need or want resource leak protection, we can later expand on this (e.g. adding an AppOption to prevent resource existence leakage and checking it to decide on the API's return). In the meantime, we should keep this simple and straightforward. # Change Type ``` /kind fix ``` # Changelog <!-- Provide the exact line to appear in release notes for the chosen changelog type. If no, just write "NONE" in the release-note block below. If yes, a release note is required: --> ```release-note Return proper API responses due to auth issues (4XX). ``` # Additional Notes <!-- Any extra context or edge cases for reviewers. --> --------- Signed-off-by: Fabian Gonzalez <[email protected]> Signed-off-by: Fabian Gonzalez <[email protected]>
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
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.
Description
Originally I had preemptively decided on returning 404s instead of actual auth errors to prevent resource leakage. I'm walking back on this decision for a few reasons now:
Actually, that's just one reason, but still important.
While the leakage prevention makes sense in public registries where any non-authenticated user can peruse, we currently do not have authn implemented that would warrant this precaution.
If we ever truly need or want resource leak protection, we can later expand on this (e.g. adding an AppOption to prevent resource existence leakage and checking it to decide on the API's return). In the meantime, we should keep this simple and straightforward.
Change Type
Changelog
Additional Notes