From 3f55b35f68a5b945fde709b5196c60a3af6fca39 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 27 Mar 2026 15:08:30 +0000 Subject: [PATCH] refactor: replace AsSystemRestricted with narrower actors (#23712) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace overly-broad `AsSystemRestricted` with purpose-built actors: - **OAuth2 provider paths** → `AsSystemOAuth2` (13 call sites across `tokens.go`, `registration.go`, `apikey.go`) - **Provisioner daemon health read** → `AsSystemReadProvisionerDaemons` (1 site in `healthcheck/provisioner.go`) - **Provisionerd file cache paths** → `AsProvisionerd` (2 sites in `provisionerdserver.go`, matching existing usage nearby)
Implementation notes Each replacement actor is a strict subset of `AsSystemRestricted`. Every DB method at each call site is already covered by the narrower actor's permissions: - `subjectSystemOAuth2`: OAuth2App/Secret/CodeToken (all), ApiKey (Read, Delete), User (Read), Organization (Read) - `subjectSystemReadProvisionerDaemons`: ProvisionerDaemon (Read) - `subjectProvisionerd`: File (Create, Read) plus provisionerd-scoped resources No new permissions added. `nolint:gocritic` comments updated to reflect the new actors.
> 🤖 Created by a Coder Agent, reviewed by me. --- coderd/healthcheck/provisioner.go | 4 +-- coderd/httpmw/apikey.go | 4 +-- coderd/oauth2provider/registration.go | 32 +++++++++---------- coderd/oauth2provider/tokens.go | 16 +++++----- .../provisionerdserver/provisionerdserver.go | 8 ++--- 5 files changed, 32 insertions(+), 32 deletions(-) diff --git a/coderd/healthcheck/provisioner.go b/coderd/healthcheck/provisioner.go index ae3220170d..ce9e4b7d39 100644 --- a/coderd/healthcheck/provisioner.go +++ b/coderd/healthcheck/provisioner.go @@ -71,8 +71,8 @@ func (r *ProvisionerDaemonsReport) Run(ctx context.Context, opts *ProvisionerDae return } - // nolint: gocritic // need an actor to fetch provisioner daemons - daemons, err := opts.Store.GetProvisionerDaemons(dbauthz.AsSystemRestricted(ctx)) + // nolint: gocritic // Read-only access to provisioner daemons for health check + daemons, err := opts.Store.GetProvisionerDaemons(dbauthz.AsSystemReadProvisionerDaemons(ctx)) if err != nil { r.Severity = health.SeverityError r.Error = ptr.Ref("error fetching provisioner daemons: " + err.Error()) diff --git a/coderd/httpmw/apikey.go b/coderd/httpmw/apikey.go index aeaf19b088..40a87647f3 100644 --- a/coderd/httpmw/apikey.go +++ b/coderd/httpmw/apikey.go @@ -699,8 +699,8 @@ func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyCon // is being used with the correct audience/resource server (RFC 8707). func validateOAuth2ProviderAppTokenAudience(ctx context.Context, db database.Store, key database.APIKey, accessURL *url.URL, r *http.Request) error { // Get the OAuth2 provider app token to check its audience - //nolint:gocritic // System needs to access token for audience validation - token, err := db.GetOAuth2ProviderAppTokenByAPIKeyID(dbauthz.AsSystemRestricted(ctx), key.ID) + //nolint:gocritic // OAuth2 system context — audience validation for provider app tokens + token, err := db.GetOAuth2ProviderAppTokenByAPIKeyID(dbauthz.AsSystemOAuth2(ctx), key.ID) if err != nil { return xerrors.Errorf("failed to get OAuth2 token: %w", err) } diff --git a/coderd/oauth2provider/registration.go b/coderd/oauth2provider/registration.go index 1891db358a..fa41023e74 100644 --- a/coderd/oauth2provider/registration.go +++ b/coderd/oauth2provider/registration.go @@ -73,8 +73,8 @@ func CreateDynamicClientRegistration(db database.Store, accessURL *url.URL, audi // Store in database - use system context since this is a public endpoint now := dbtime.Now() clientName := req.GenerateClientName() - //nolint:gocritic // Dynamic client registration is a public endpoint, system access required - app, err := db.InsertOAuth2ProviderApp(dbauthz.AsSystemRestricted(ctx), database.InsertOAuth2ProviderAppParams{ + //nolint:gocritic // OAuth2 system context — dynamic registration is a public endpoint + app, err := db.InsertOAuth2ProviderApp(dbauthz.AsSystemOAuth2(ctx), database.InsertOAuth2ProviderAppParams{ ID: clientID, CreatedAt: now, UpdatedAt: now, @@ -121,8 +121,8 @@ func CreateDynamicClientRegistration(db database.Store, accessURL *url.URL, audi return } - //nolint:gocritic // Dynamic client registration is a public endpoint, system access required - _, err = db.InsertOAuth2ProviderAppSecret(dbauthz.AsSystemRestricted(ctx), database.InsertOAuth2ProviderAppSecretParams{ + //nolint:gocritic // OAuth2 system context — dynamic registration is a public endpoint + _, err = db.InsertOAuth2ProviderAppSecret(dbauthz.AsSystemOAuth2(ctx), database.InsertOAuth2ProviderAppSecretParams{ ID: uuid.New(), CreatedAt: now, SecretPrefix: []byte(parsedSecret.Prefix), @@ -183,8 +183,8 @@ func GetClientConfiguration(db database.Store) http.HandlerFunc { } // Get app by client ID - //nolint:gocritic // RFC 7592 endpoints need system access to retrieve dynamically registered clients - app, err := db.GetOAuth2ProviderAppByClientID(dbauthz.AsSystemRestricted(ctx), clientID) + //nolint:gocritic // OAuth2 system context — RFC 7592 client configuration endpoint + app, err := db.GetOAuth2ProviderAppByClientID(dbauthz.AsSystemOAuth2(ctx), clientID) if err != nil { if xerrors.Is(err, sql.ErrNoRows) { writeOAuth2RegistrationError(ctx, rw, http.StatusUnauthorized, @@ -269,8 +269,8 @@ func UpdateClientConfiguration(db database.Store, auditor *audit.Auditor, logger req = req.ApplyDefaults() // Get existing app to verify it exists and is dynamically registered - //nolint:gocritic // RFC 7592 endpoints need system access to retrieve dynamically registered clients - existingApp, err := db.GetOAuth2ProviderAppByClientID(dbauthz.AsSystemRestricted(ctx), clientID) + //nolint:gocritic // OAuth2 system context — RFC 7592 client configuration endpoint + existingApp, err := db.GetOAuth2ProviderAppByClientID(dbauthz.AsSystemOAuth2(ctx), clientID) if err == nil { aReq.Old = existingApp } @@ -294,8 +294,8 @@ func UpdateClientConfiguration(db database.Store, auditor *audit.Auditor, logger // Update app in database now := dbtime.Now() - //nolint:gocritic // RFC 7592 endpoints need system access to update dynamically registered clients - updatedApp, err := db.UpdateOAuth2ProviderAppByClientID(dbauthz.AsSystemRestricted(ctx), database.UpdateOAuth2ProviderAppByClientIDParams{ + //nolint:gocritic // OAuth2 system context — RFC 7592 client configuration endpoint + updatedApp, err := db.UpdateOAuth2ProviderAppByClientID(dbauthz.AsSystemOAuth2(ctx), database.UpdateOAuth2ProviderAppByClientIDParams{ ID: clientID, UpdatedAt: now, Name: req.GenerateClientName(), @@ -377,8 +377,8 @@ func DeleteClientConfiguration(db database.Store, auditor *audit.Auditor, logger } // Get existing app to verify it exists and is dynamically registered - //nolint:gocritic // RFC 7592 endpoints need system access to retrieve dynamically registered clients - existingApp, err := db.GetOAuth2ProviderAppByClientID(dbauthz.AsSystemRestricted(ctx), clientID) + //nolint:gocritic // OAuth2 system context — RFC 7592 client configuration endpoint + existingApp, err := db.GetOAuth2ProviderAppByClientID(dbauthz.AsSystemOAuth2(ctx), clientID) if err == nil { aReq.Old = existingApp } @@ -401,8 +401,8 @@ func DeleteClientConfiguration(db database.Store, auditor *audit.Auditor, logger } // Delete the client and all associated data (tokens, secrets, etc.) - //nolint:gocritic // RFC 7592 endpoints need system access to delete dynamically registered clients - err = db.DeleteOAuth2ProviderAppByClientID(dbauthz.AsSystemRestricted(ctx), clientID) + //nolint:gocritic // OAuth2 system context — RFC 7592 client configuration endpoint + err = db.DeleteOAuth2ProviderAppByClientID(dbauthz.AsSystemOAuth2(ctx), clientID) if err != nil { writeOAuth2RegistrationError(ctx, rw, http.StatusInternalServerError, "server_error", "Failed to delete client") @@ -453,8 +453,8 @@ func RequireRegistrationAccessToken(db database.Store) func(http.Handler) http.H } // Get the client and verify the registration access token - //nolint:gocritic // RFC 7592 endpoints need system access to validate dynamically registered clients - app, err := db.GetOAuth2ProviderAppByClientID(dbauthz.AsSystemRestricted(ctx), clientID) + //nolint:gocritic // OAuth2 system context — RFC 7592 registration access token validation + app, err := db.GetOAuth2ProviderAppByClientID(dbauthz.AsSystemOAuth2(ctx), clientID) if err != nil { if xerrors.Is(err, sql.ErrNoRows) { // Return 401 for authentication-related issues, not 404 diff --git a/coderd/oauth2provider/tokens.go b/coderd/oauth2provider/tokens.go index 8380d307a5..638856d3e6 100644 --- a/coderd/oauth2provider/tokens.go +++ b/coderd/oauth2provider/tokens.go @@ -217,8 +217,8 @@ func authorizationCodeGrant(ctx context.Context, db database.Store, app database if err != nil { return codersdk.OAuth2TokenResponse{}, errBadSecret } - //nolint:gocritic // Users cannot read secrets so we must use the system. - dbSecret, err := db.GetOAuth2ProviderAppSecretByPrefix(dbauthz.AsSystemRestricted(ctx), []byte(secret.Prefix)) + //nolint:gocritic // OAuth2 system context — users cannot read secrets + dbSecret, err := db.GetOAuth2ProviderAppSecretByPrefix(dbauthz.AsSystemOAuth2(ctx), []byte(secret.Prefix)) if errors.Is(err, sql.ErrNoRows) { return codersdk.OAuth2TokenResponse{}, errBadSecret } @@ -236,8 +236,8 @@ func authorizationCodeGrant(ctx context.Context, db database.Store, app database if err != nil { return codersdk.OAuth2TokenResponse{}, errBadCode } - //nolint:gocritic // There is no user yet so we must use the system. - dbCode, err := db.GetOAuth2ProviderAppCodeByPrefix(dbauthz.AsSystemRestricted(ctx), []byte(code.Prefix)) + //nolint:gocritic // OAuth2 system context — no authenticated user during token exchange + dbCode, err := db.GetOAuth2ProviderAppCodeByPrefix(dbauthz.AsSystemOAuth2(ctx), []byte(code.Prefix)) if errors.Is(err, sql.ErrNoRows) { return codersdk.OAuth2TokenResponse{}, errBadCode } @@ -384,8 +384,8 @@ func refreshTokenGrant(ctx context.Context, db database.Store, app database.OAut if err != nil { return codersdk.OAuth2TokenResponse{}, errBadToken } - //nolint:gocritic // There is no user yet so we must use the system. - dbToken, err := db.GetOAuth2ProviderAppTokenByPrefix(dbauthz.AsSystemRestricted(ctx), []byte(token.Prefix)) + //nolint:gocritic // OAuth2 system context — no authenticated user during refresh + dbToken, err := db.GetOAuth2ProviderAppTokenByPrefix(dbauthz.AsSystemOAuth2(ctx), []byte(token.Prefix)) if errors.Is(err, sql.ErrNoRows) { return codersdk.OAuth2TokenResponse{}, errBadToken } @@ -411,8 +411,8 @@ func refreshTokenGrant(ctx context.Context, db database.Store, app database.OAut } // Grab the user roles so we can perform the refresh as the user. - //nolint:gocritic // There is no user yet so we must use the system. - prevKey, err := db.GetAPIKeyByID(dbauthz.AsSystemRestricted(ctx), dbToken.APIKeyID) + //nolint:gocritic // OAuth2 system context — need to read the previous API key + prevKey, err := db.GetAPIKeyByID(dbauthz.AsSystemOAuth2(ctx), dbToken.APIKeyID) if err != nil { return codersdk.OAuth2TokenResponse{}, err } diff --git a/coderd/provisionerdserver/provisionerdserver.go b/coderd/provisionerdserver/provisionerdserver.go index ed12ca2798..c663e0e7e6 100644 --- a/coderd/provisionerdserver/provisionerdserver.go +++ b/coderd/provisionerdserver/provisionerdserver.go @@ -1881,8 +1881,8 @@ func (s *server) completeTemplateImportJob(ctx context.Context, job database.Pro hashBytes := sha256.Sum256(moduleFiles) hash := hex.EncodeToString(hashBytes[:]) - // nolint:gocritic // Requires reading "system" files - file, err := db.GetFileByHashAndCreator(dbauthz.AsSystemRestricted(ctx), database.GetFileByHashAndCreatorParams{Hash: hash, CreatedBy: uuid.Nil}) + //nolint:gocritic // Acting as provisionerd + file, err := db.GetFileByHashAndCreator(dbauthz.AsProvisionerd(ctx), database.GetFileByHashAndCreatorParams{Hash: hash, CreatedBy: uuid.Nil}) switch { case err == nil: // This set of modules is already cached, which means we can reuse them @@ -1893,8 +1893,8 @@ func (s *server) completeTemplateImportJob(ctx context.Context, job database.Pro case !xerrors.Is(err, sql.ErrNoRows): return xerrors.Errorf("check for cached modules: %w", err) default: - // nolint:gocritic // Requires creating a "system" file - file, err = db.InsertFile(dbauthz.AsSystemRestricted(ctx), database.InsertFileParams{ + //nolint:gocritic // Acting as provisionerd + file, err = db.InsertFile(dbauthz.AsProvisionerd(ctx), database.InsertFileParams{ ID: uuid.New(), Hash: hash, CreatedBy: uuid.Nil,