From f514ec905f7986df2ab24f19cb7f4022379ecaf1 Mon Sep 17 00:00:00 2001 From: logikonline Date: Sat, 24 Jan 2026 14:50:56 -0500 Subject: [PATCH] feat(secrets): add global admin-managed secrets Implements system-wide global secrets (OwnerID=0, RepoID=0) that can only be managed by admins. Global secrets are available to all workflows with lowest precedence (repo > org > global). Adds admin UI routes and templates for managing global secrets. Updates secret model to support three-tier hierarchy and proper precedence ordering. --- .notes/note-1769283562795-n834eh806.json | 8 ++++ models/secret/secret.go | 45 +++++++++++++++++------ routers/web/repo/setting/secrets.go | 28 +++++++++++--- routers/web/shared/secrets/secrets.go | 30 +++++++++++++-- routers/web/web.go | 1 + services/secrets/secrets.go | 47 ++++++++++++++++++++++++ templates/admin/actions.tmpl | 3 ++ templates/admin/navbar.tmpl | 5 ++- 8 files changed, 145 insertions(+), 22 deletions(-) create mode 100644 .notes/note-1769283562795-n834eh806.json diff --git a/.notes/note-1769283562795-n834eh806.json b/.notes/note-1769283562795-n834eh806.json new file mode 100644 index 0000000000..7c2bb62ec4 --- /dev/null +++ b/.notes/note-1769283562795-n834eh806.json @@ -0,0 +1,8 @@ +{ + "id": "note-1769283562795-n834eh806", + "title": "translate", + "content": " \"packages.visibility\": \"Visibility\",\n \"packages.settings.visibility.private.text\": \"This package is currently private. Make it public to allow anyone to access it.\",\n \"packages.settings.visibility.private.button\": \"Make Private\",\n \"packages.settings.visibility.private.bullet_title\": \"You are about to make this package private.\",\n \"packages.settings.visibility.private.bullet_one\": \"Only users with appropriate permissions will be able to access this package.\",\n \"packages.settings.visibility.private.success\": \"Package is now private.\",\n \"packages.settings.visibility.public.text\": \"This package is currently public. Make it private to restrict access.\",\n \"packages.settings.visibility.public.button\": \"Make Public\",\n \"packages.settings.visibility.public.bullet_title\": \"You are about to make this package public.\",\n \"packages.settings.visibility.public.bullet_one\": \"Anyone will be able to access and download this package.\",\n \"packages.settings.visibility.public.success\": \"Package is now public.\",\n \"packages.settings.visibility.error\": \"Failed to update package visibility.\",", + "createdAt": 1769283562793, + "updatedAt": 1769283566248, + "tags": [] +} \ No newline at end of file diff --git a/models/secret/secret.go b/models/secret/secret.go index 2a51781f8d..57c665a877 100644 --- a/models/secret/secret.go +++ b/models/secret/secret.go @@ -23,8 +23,9 @@ import ( // Secret represents a secret // // It can be: -// 1. org/user level secret, OwnerID is org/user ID and RepoID is 0 -// 2. repo level secret, OwnerID is 0 and RepoID is repo ID +// 1. global/system level secret, OwnerID is 0 and RepoID is 0 (admin only) +// 2. org/user level secret, OwnerID is org/user ID and RepoID is 0 +// 3. repo level secret, OwnerID is 0 and RepoID is repo ID // // Please note that it's not acceptable to have both OwnerID and RepoID to be non-zero, // or it will be complicated to find secrets belonging to a specific owner. @@ -32,8 +33,7 @@ import ( // but it's a repo level secret, not an org/user level secret. // To avoid this, make it clear with {OwnerID: 0, RepoID: 1} for repo level secrets. // -// Please note that it's not acceptable to have both OwnerID and RepoID to zero, global secrets are not supported. -// It's for security reasons, admin may be not aware of that the secrets could be stolen by any user when setting them as global. +// Global secrets (OwnerID=0, RepoID=0) are available to all workflows and can only be managed by admins. type Secret struct { ID int64 OwnerID int64 `xorm:"INDEX UNIQUE(owner_repo_name) NOT NULL"` @@ -63,15 +63,14 @@ func (err ErrSecretNotFound) Unwrap() error { } // InsertEncryptedSecret Creates, encrypts, and validates a new secret with yet unencrypted data and insert into database +// Note: Global secrets (ownerID=0, repoID=0) are allowed and can only be managed by admins func InsertEncryptedSecret(ctx context.Context, ownerID, repoID int64, name, data, description string) (*Secret, error) { if ownerID != 0 && repoID != 0 { // It's trying to create a secret that belongs to a repository, but OwnerID has been set accidentally. // Remove OwnerID to avoid confusion; it's not worth returning an error here. ownerID = 0 } - if ownerID == 0 && repoID == 0 { - return nil, fmt.Errorf("%w: ownerID and repoID cannot be both zero, global secrets are not supported", util.ErrInvalidArgument) - } + // Global secrets (ownerID=0, repoID=0) are now allowed for admin-managed system-wide secrets if len(data) > SecretDataMaxLength { return nil, util.NewInvalidArgumentErrorf("data too long") @@ -104,17 +103,24 @@ type FindSecretsOptions struct { OwnerID int64 // it will be ignored if RepoID is set SecretID int64 Name string + Global bool // if true, search for global secrets (OwnerID=0, RepoID=0) } func (opts FindSecretsOptions) ToConds() builder.Cond { cond := builder.NewCond() - cond = cond.And(builder.Eq{"repo_id": opts.RepoID}) - if opts.RepoID != 0 { // if RepoID is set - // ignore OwnerID and treat it as 0 + if opts.Global { + // Global secrets have both OwnerID=0 and RepoID=0 cond = cond.And(builder.Eq{"owner_id": 0}) + cond = cond.And(builder.Eq{"repo_id": 0}) } else { - cond = cond.And(builder.Eq{"owner_id": opts.OwnerID}) + cond = cond.And(builder.Eq{"repo_id": opts.RepoID}) + if opts.RepoID != 0 { // if RepoID is set + // ignore OwnerID and treat it as 0 + cond = cond.And(builder.Eq{"owner_id": 0}) + } else { + cond = cond.And(builder.Eq{"owner_id": opts.OwnerID}) + } } if opts.SecretID != 0 { @@ -164,6 +170,15 @@ func GetSecretsOfTask(ctx context.Context, task *actions_model.ActionTask) (map[ return secrets, nil } + // Load secrets in order of precedence (later overrides earlier): + // 1. Global secrets (system-wide, admin-managed) + // 2. Owner/org secrets + // 3. Repository secrets (most specific, highest precedence) + globalSecrets, err := db.Find[Secret](ctx, FindSecretsOptions{Global: true}) + if err != nil { + log.Error("find global secrets: %v", err) + return nil, err + } ownerSecrets, err := db.Find[Secret](ctx, FindSecretsOptions{OwnerID: task.Job.Run.Repo.OwnerID}) if err != nil { log.Error("find secrets of owner %v: %v", task.Job.Run.Repo.OwnerID, err) @@ -175,7 +190,13 @@ func GetSecretsOfTask(ctx context.Context, task *actions_model.ActionTask) (map[ return nil, err } - for _, secret := range append(ownerSecrets, repoSecrets...) { + // Add secrets in order: global, then owner, then repo (repo overrides owner overrides global) + allSecrets := make([]*Secret, 0, len(globalSecrets)+len(ownerSecrets)+len(repoSecrets)) + allSecrets = append(allSecrets, globalSecrets...) + allSecrets = append(allSecrets, ownerSecrets...) + allSecrets = append(allSecrets, repoSecrets...) + + for _, secret := range allSecrets { v, err := secret_module.DecryptSecret(setting.SecretKey, secret.Data) if err != nil { log.Error("Unable to decrypt Actions secret %v %q, maybe SECRET_KEY is wrong: %v", secret.ID, secret.Name, err) diff --git a/routers/web/repo/setting/secrets.go b/routers/web/repo/setting/secrets.go index e46ba66586..326480e339 100644 --- a/routers/web/repo/setting/secrets.go +++ b/routers/web/repo/setting/secrets.go @@ -17,9 +17,10 @@ import ( const ( // TODO: Separate secrets from runners when layout is ready - tplRepoSecrets templates.TplName = "repo/settings/actions" - tplOrgSecrets templates.TplName = "org/settings/actions" - tplUserSecrets templates.TplName = "user/settings/actions" + tplRepoSecrets templates.TplName = "repo/settings/actions" + tplOrgSecrets templates.TplName = "org/settings/actions" + tplUserSecrets templates.TplName = "user/settings/actions" + tplAdminSecrets templates.TplName = "admin/actions" ) type secretsCtx struct { @@ -28,6 +29,8 @@ type secretsCtx struct { IsRepo bool IsOrg bool IsUser bool + IsAdmin bool + IsGlobal bool SecretsTemplate templates.TplName RedirectLink string } @@ -67,6 +70,17 @@ func getSecretsCtx(ctx *context.Context) (*secretsCtx, error) { }, nil } + if ctx.Data["PageIsAdmin"] == true { + return &secretsCtx{ + OwnerID: 0, + RepoID: 0, + IsAdmin: true, + IsGlobal: true, + SecretsTemplate: tplAdminSecrets, + RedirectLink: setting.AppSubURL + "/-/admin/actions/secrets", + }, nil + } + return nil, errors.New("unable to set Secrets context") } @@ -86,7 +100,7 @@ func Secrets(ctx *context.Context) { ctx.Data["DisableSSH"] = setting.SSH.Disabled } - shared.SetSecretsContext(ctx, sCtx.OwnerID, sCtx.RepoID) + shared.SetSecretsContextWithGlobal(ctx, sCtx.OwnerID, sCtx.RepoID, sCtx.IsGlobal) if ctx.Written() { return } @@ -105,10 +119,11 @@ func SecretsPost(ctx *context.Context) { return } - shared.PerformSecretsPost( + shared.PerformSecretsPostWithGlobal( ctx, sCtx.OwnerID, sCtx.RepoID, + sCtx.IsGlobal, sCtx.RedirectLink, ) } @@ -119,10 +134,11 @@ func SecretsDelete(ctx *context.Context) { ctx.ServerError("getSecretsCtx", err) return } - shared.PerformSecretsDelete( + shared.PerformSecretsDeleteWithGlobal( ctx, sCtx.OwnerID, sCtx.RepoID, + sCtx.IsGlobal, sCtx.RedirectLink, ) } diff --git a/routers/web/shared/secrets/secrets.go b/routers/web/shared/secrets/secrets.go index bb2900433f..1df9f32e46 100644 --- a/routers/web/shared/secrets/secrets.go +++ b/routers/web/shared/secrets/secrets.go @@ -15,7 +15,12 @@ import ( ) func SetSecretsContext(ctx *context.Context, ownerID, repoID int64) { - secrets, err := db.Find[secret_model.Secret](ctx, secret_model.FindSecretsOptions{OwnerID: ownerID, RepoID: repoID}) + SetSecretsContextWithGlobal(ctx, ownerID, repoID, false) +} + +func SetSecretsContextWithGlobal(ctx *context.Context, ownerID, repoID int64, global bool) { + opts := secret_model.FindSecretsOptions{OwnerID: ownerID, RepoID: repoID, Global: global} + secrets, err := db.Find[secret_model.Secret](ctx, opts) if err != nil { ctx.ServerError("FindSecrets", err) return @@ -27,9 +32,19 @@ func SetSecretsContext(ctx *context.Context, ownerID, repoID int64) { } func PerformSecretsPost(ctx *context.Context, ownerID, repoID int64, redirectURL string) { + PerformSecretsPostWithGlobal(ctx, ownerID, repoID, false, redirectURL) +} + +func PerformSecretsPostWithGlobal(ctx *context.Context, ownerID, repoID int64, global bool, redirectURL string) { form := web.GetForm(ctx).(*forms.AddSecretForm) - s, _, err := secret_service.CreateOrUpdateSecret(ctx, ownerID, repoID, form.Name, util.ReserveLineBreakForTextarea(form.Data), form.Description) + var s *secret_model.Secret + var err error + if global { + s, _, err = secret_service.CreateOrUpdateGlobalSecret(ctx, form.Name, util.ReserveLineBreakForTextarea(form.Data), form.Description) + } else { + s, _, err = secret_service.CreateOrUpdateSecret(ctx, ownerID, repoID, form.Name, util.ReserveLineBreakForTextarea(form.Data), form.Description) + } if err != nil { log.Error("CreateOrUpdateSecret failed: %v", err) ctx.JSONError(ctx.Tr("secrets.save_failed")) @@ -41,9 +56,18 @@ func PerformSecretsPost(ctx *context.Context, ownerID, repoID int64, redirectURL } func PerformSecretsDelete(ctx *context.Context, ownerID, repoID int64, redirectURL string) { + PerformSecretsDeleteWithGlobal(ctx, ownerID, repoID, false, redirectURL) +} + +func PerformSecretsDeleteWithGlobal(ctx *context.Context, ownerID, repoID int64, global bool, redirectURL string) { id := ctx.FormInt64("id") - err := secret_service.DeleteSecretByID(ctx, ownerID, repoID, id) + var err error + if global { + err = secret_service.DeleteGlobalSecretByID(ctx, id) + } else { + err = secret_service.DeleteSecretByID(ctx, ownerID, repoID, id) + } if err != nil { log.Error("DeleteSecretByID(%d) failed: %v", id, err) ctx.JSONError(ctx.Tr("secrets.deletion.failed")) diff --git a/routers/web/web.go b/routers/web/web.go index 13d10eb5c4..837073f856 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -887,6 +887,7 @@ func registerWebRoutes(m *web.Router) { m.Group("/actions", func() { m.Get("", admin.RedirectToDefaultSetting) addSettingsRunnersRoutes() + addSettingsSecretsRoutes() addSettingsVariablesRoutes() }) }, adminReq, ctxDataSet("EnableOAuth2", setting.OAuth2.Enabled, "EnablePackages", setting.Packages.Enabled)) diff --git a/services/secrets/secrets.go b/services/secrets/secrets.go index 4e3ba6acdc..0919b57c63 100644 --- a/services/secrets/secrets.go +++ b/services/secrets/secrets.go @@ -39,6 +39,37 @@ func CreateOrUpdateSecret(ctx context.Context, ownerID, repoID int64, name, data return s[0], false, nil } +// CreateOrUpdateGlobalSecret creates or updates a global secret (OwnerID=0, RepoID=0) +// Global secrets are available to all workflows system-wide +func CreateOrUpdateGlobalSecret(ctx context.Context, name, data, description string) (*secret_model.Secret, bool, error) { + if err := ValidateName(name); err != nil { + return nil, false, err + } + + s, err := db.Find[secret_model.Secret](ctx, secret_model.FindSecretsOptions{ + Global: true, + Name: name, + }) + if err != nil { + return nil, false, err + } + + if len(s) == 0 { + // Insert with ownerID=0, repoID=0 for global secret + s, err := secret_model.InsertEncryptedSecret(ctx, 0, 0, name, data, description) + if err != nil { + return nil, false, err + } + return s, true, nil + } + + if err := secret_model.UpdateSecret(ctx, s[0].ID, data, description); err != nil { + return nil, false, err + } + + return s[0], false, nil +} + func DeleteSecretByID(ctx context.Context, ownerID, repoID, secretID int64) error { s, err := db.Find[secret_model.Secret](ctx, secret_model.FindSecretsOptions{ OwnerID: ownerID, @@ -55,6 +86,22 @@ func DeleteSecretByID(ctx context.Context, ownerID, repoID, secretID int64) erro return deleteSecret(ctx, s[0]) } +// DeleteGlobalSecretByID deletes a global secret by ID +func DeleteGlobalSecretByID(ctx context.Context, secretID int64) error { + s, err := db.Find[secret_model.Secret](ctx, secret_model.FindSecretsOptions{ + Global: true, + SecretID: secretID, + }) + if err != nil { + return err + } + if len(s) != 1 { + return secret_model.ErrSecretNotFound{} + } + + return deleteSecret(ctx, s[0]) +} + func DeleteSecretByName(ctx context.Context, ownerID, repoID int64, name string) error { s, err := db.Find[secret_model.Secret](ctx, secret_model.FindSecretsOptions{ OwnerID: ownerID, diff --git a/templates/admin/actions.tmpl b/templates/admin/actions.tmpl index 597863d73b..e5cad8ecf4 100644 --- a/templates/admin/actions.tmpl +++ b/templates/admin/actions.tmpl @@ -3,6 +3,9 @@ {{if eq .PageType "runners"}} {{template "shared/actions/runner_list" .}} {{end}} + {{if eq .PageType "secrets"}} + {{template "shared/secrets/add_list" .}} + {{end}} {{if eq .PageType "variables"}} {{template "shared/variables/variable_list" .}} {{end}} diff --git a/templates/admin/navbar.tmpl b/templates/admin/navbar.tmpl index a71eade31f..5ae5571da3 100644 --- a/templates/admin/navbar.tmpl +++ b/templates/admin/navbar.tmpl @@ -69,12 +69,15 @@ {{end}} {{end}} {{if .EnableActions}} -
+
{{ctx.Locale.Tr "actions.actions"}}