Skip to content

fix: set all api endpoints to use auth by default and explicitly remove auth for public endpoints#2377

Merged
kmendell merged 1 commit intomainfrom
fix/auth-by-default
Apr 15, 2026
Merged

fix: set all api endpoints to use auth by default and explicitly remove auth for public endpoints#2377
kmendell merged 1 commit intomainfrom
fix/auth-by-default

Conversation

@kmendell
Copy link
Copy Markdown
Member

@kmendell kmendell commented Apr 15, 2026

Checklist

  • This PR is not opened from my fork’s main branch

What This PR Implements

Fixes:

Changes Made

Testing Done

  • Development environment started: ./scripts/development/dev.sh start
  • Frontend verified at http://localhost:3000
  • Backend verified at http://localhost:3552
  • Manual testing completed (describe):
  • No linting errors (e.g., just lint all)
  • Backend tests pass: just test backend

AI Tool Used (if applicable)

AI Tool:
Assistance Level:
What AI helped with:
I reviewed and edited all AI-generated output:
I ran all required tests and manually verified changes:

Additional Context

Disclaimer Greptiles Reviews use AI, make sure to check over its work.

To better help train Greptile on our codebase, if the comment is useful and valid Like the comment, if its not helpful or invalid Dislike

To have Greptile Re-Review the changes, mention greptileai.

Greptile Summary

This PR implements "auth by default" by configuring global humaConfig.Security with BearerAuth and ApiKeyAuth, so any operation that doesn't explicitly override its Security field inherits the requirement. Public endpoints (health, login, OIDC flows, app images, public settings, environment pairing) correctly opt out with Security: []map[string][]string{}. Template read endpoints now intentionally require authentication by relying on global inheritance, confirmed by the new TestSetupAPIForSpec_TemplateReadRoutesProtected test. The settings model gains a new SettingVisibilityNonAdmin tier, surfacing only public- and authrequired-tagged fields to authenticated non-admin users.

Confidence Score: 5/5

Safe to merge — auth-by-default logic is correct and well-tested; only a naming convention P2 remains.

All remaining findings are P2 style suggestions (unexported function naming convention). The core security logic — global security inheritance, public endpoint opt-out, visibility tier filtering — is sound and covered by new tests.

backend/internal/huma/middleware/auth.go — naming convention; no functional issues.

Comments Outside Diff (1)

  1. backend/internal/huma/handlers/templates.go, line 199-209 (link)

    P2 Redundant explicit security on template handlers

    The four template read handlers (listTemplates, getAllTemplates, getTemplate, getTemplateContent) now carry an explicit Security block that is byte-for-byte identical to the global API security set in huma.go. Because any operation that omits Security already inherits the global default via parseSecurityRequirements, these blocks add noise without changing behaviour. Removing them would reduce boilerplate and make future global-security changes automatic for these routes.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: backend/internal/huma/handlers/templates.go
    Line: 199-209
    
    Comment:
    **Redundant explicit security on template handlers**
    
    The four template read handlers (`listTemplates`, `getAllTemplates`, `getTemplate`, `getTemplateContent`) now carry an explicit `Security` block that is byte-for-byte identical to the global API security set in `huma.go`. Because any operation that omits `Security` already inherits the global default via `parseSecurityRequirements`, these blocks add noise without changing behaviour. Removing them would reduce boilerplate and make future global-security changes automatic for these routes.
    
    How can I resolve this? If you propose a fix, please make it concise.

    Fix in Codex

Fix All in Codex

Prompt To Fix All With AI
This is a comment left during a code review.
Path: backend/internal/huma/middleware/auth.go
Line: 64-95

Comment:
**Unexported functions missing "Internal" suffix**

Several unexported functions added in this file don't follow the project-wide convention requiring the `Internal` suffix on all unexported functions. `parseSecurityRequirements` (the core of this PR's auth-by-default mechanism) and the other helpers should all carry the suffix.

Functions affected in `middleware/auth.go`: `parseSecurityRequirements`, `tryBearerAuth`, `tryApiKeyAuth`, `tryAgentAuth`, `createAgentSudoUser`, `extractBearerToken`, `extractTokenFromCookieHeader`, `setUserInContext`. The same pattern appears in the new unexported helpers added to `handlers/settings.go` (`validateProjectsDirectoryValue`, `validateAbsoluteDirectoryPath`, `hasAuthSettingsUpdate`).

**Rule Used:** What: All unexported functions must have the "Inte... ([source](https://app.greptile.com/review/custom-context?memory=306fc233-4d2f-4ac4-bdf7-8059588e8a43))

How can I resolve this? If you propose a fix, please make it concise.

Reviews (2): Last reviewed commit: "fix: set all api endpoints to use auth b..." | Re-trigger Greptile

Context used:

  • Rule used - What: All unexported functions must have the "Inte... (source)

@kmendell kmendell marked this pull request as ready for review April 15, 2026 22:03
@kmendell kmendell requested a review from a team April 15, 2026 22:03
@kmendell
Copy link
Copy Markdown
Member Author

kmendell commented Apr 15, 2026

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

Copy link
Copy Markdown
Member Author

@getarcaneappbot
Copy link
Copy Markdown
Contributor

getarcaneappbot commented Apr 15, 2026

Container images for this PR have been built successfully!

  • Manager: ghcr.io/getarcaneapp/arcane:pr-2377
  • Agent: ghcr.io/getarcaneapp/arcane-headless:pr-2377

Built from commit 603f93a

Comment thread backend/internal/models/settings.go Outdated
Comment thread backend/internal/huma/handlers/settings.go
@kmendell kmendell force-pushed the fix/auth-by-default branch from 3187920 to 3c1e790 Compare April 15, 2026 22:41
@kmendell kmendell force-pushed the fix/auth-by-default branch from 3c1e790 to 603f93a Compare April 15, 2026 22:51
@kmendell kmendell merged commit 8aa6153 into main Apr 15, 2026
23 checks passed
@kmendell kmendell deleted the fix/auth-by-default branch April 15, 2026 22:53
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