From 9c5b278622c5b7f9d3e78d533587c6f29e20bb8f Mon Sep 17 00:00:00 2001 From: Excellencedev Date: Sat, 20 Dec 2025 05:46:33 +0100 Subject: [PATCH] Fix all bugs I found in the code --- models/perm/access/repo_permission.go | 9 ++- options/locale/locale_en-US.json | 6 ++ routers/api/packages/api.go | 77 ++++++++++---------- routers/web/org/setting/actions.go | 23 +++--- routers/web/repo/setting/actions.go | 23 +++--- templates/org/settings/actions_general.tmpl | 49 ++++++++----- templates/repo/settings/actions_general.tmpl | 14 ++-- tests/integration/actions_job_token_test.go | 6 +- web_src/js/features/repo-settings-actions.ts | 26 +------ 9 files changed, 127 insertions(+), 106 deletions(-) diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go index b945ff8a74..c934a13d03 100644 --- a/models/perm/access/repo_permission.go +++ b/models/perm/access/repo_permission.go @@ -272,7 +272,14 @@ func GetActionsUserRepoPermission(ctx context.Context, repo *repo_model.Reposito return perm, err } - actionsUnit := repo.MustGetUnit(ctx, unit.TypeActions) + actionsUnit, err := repo.GetUnit(ctx, unit.TypeActions) + if err != nil { + // If Actions unit doesn't exist, return empty permission + if repo_model.IsErrUnitTypeNotExist(err) { + return perm, nil + } + return perm, err + } actionsCfg := actionsUnit.ActionsConfig() if task.RepoID != repo.ID { diff --git a/options/locale/locale_en-US.json b/options/locale/locale_en-US.json index 819542ace1..8f420b9581 100644 --- a/options/locale/locale_en-US.json +++ b/options/locale/locale_en-US.json @@ -3779,11 +3779,17 @@ "general.token_permissions.restricted.description": "Workflows have read permissions in the repository for the contents and packages scopes only.", "general.token_permissions.fork_pr_note": "Note: For workflows triggered by a pull request from a forked repository, the default GITHUB_TOKEN is always read-only.", "general.token_permissions.contents": "Contents", + "general.token_permissions.contents.description": "Access source code, files, commits and branches.", "general.token_permissions.issues": "Issues", + "general.token_permissions.issues.description": "Organize bug reports, tasks and milestones.", "general.token_permissions.pull_requests": "Pull Requests", + "general.token_permissions.pull_requests.description": "Enable pull requests and code reviews.", "general.token_permissions.packages": "Packages", + "general.token_permissions.packages.description": "Manage repository packages.", "general.token_permissions.actions_scope": "Actions", + "general.token_permissions.actions_scope.description": "Manage actions.", "general.token_permissions.wiki": "Wiki", + "general.token_permissions.wiki.description": "Write and share documentation with collaborators.", "general.token_permissions.access_read": "Read", "general.token_permissions.access_write": "Write", "general.token_permissions.access_none": "No access", diff --git a/routers/api/packages/api.go b/routers/api/packages/api.go index 32195cea99..1d1ae4291e 100644 --- a/routers/api/packages/api.go +++ b/routers/api/packages/api.go @@ -81,51 +81,54 @@ func reqPackageAccess(accessMode perm.AccessMode) func(ctx *context.Context) { } } - if ctx.Data["IsActionsToken"] == true { - if ctx.Package != nil && ctx.Package.Owner.Visibility.IsPrivate() { - // Actions rules: - // 1. If the package key matches the task repo, allow. - // 2. If not, check cross-repo policy. + isActionsToken, _ := ctx.Data["IsActionsToken"].(bool) + if isActionsToken && ctx.Package != nil && ctx.Package.Owner != nil && ctx.Package.Owner.Visibility.IsPrivate() { + // Actions rules: + // 1. If the package key matches the task repo, allow. + // 2. If not, check cross-repo policy. - taskID, ok := ctx.Data["ActionsTaskID"].(int64) - if ok { - task, err := actions_model.GetTaskByID(ctx, taskID) - if err != nil { - log.Error("GetTaskByID: %v", err) - ctx.HTTPError(http.StatusInternalServerError, "GetTaskByID", err.Error()) + taskID, ok := ctx.Data["ActionsTaskID"].(int64) + if ok && taskID > 0 { + task, err := actions_model.GetTaskByID(ctx, taskID) + if err != nil { + log.Error("GetTaskByID: %v", err) + ctx.HTTPError(http.StatusInternalServerError, "GetTaskByID", err.Error()) + return + } + if task == nil { + ctx.HTTPError(http.StatusInternalServerError, "GetTaskByID", "task not found") + return + } + + var packageRepoID int64 + if ctx.Package.Descriptor != nil && ctx.Package.Descriptor.Package != nil { + packageRepoID = ctx.Package.Descriptor.Package.RepoID + } + + if task.RepoID != packageRepoID { + // 1. Private packages MUST be linked to a repository + if packageRepoID == 0 { + ctx.HTTPError(http.StatusForbidden, "reqPackageAccess", "private package must be linked to a repository to be accessed by Actions") return } - var packageRepoID int64 - if ctx.Package.Descriptor != nil && ctx.Package.Descriptor.Package != nil { - packageRepoID = ctx.Package.Descriptor.Package.RepoID - } - - if task.RepoID != packageRepoID { - // 1. Private packages MUST be linked to a repository - if packageRepoID == 0 { - ctx.HTTPError(http.StatusForbidden, "reqPackageAccess", "private package must be linked to a repository to be accessed by Actions") + // 2. Check Org Cross-Repo Access Policy + if ctx.Package.Owner.IsOrganization() { + cfg, err := actions_model.GetOrgActionsConfig(ctx, ctx.Package.Owner.ID) + if err != nil { + log.Error("GetOrgActionsConfig: %v", err) + ctx.HTTPError(http.StatusInternalServerError, "GetOrgActionsConfig", err.Error()) return } - - // 2. Check Org Cross-Repo Access Policy - if ctx.Package.Owner.IsOrganization() { - cfg, err := actions_model.GetOrgActionsConfig(ctx, ctx.Package.Owner.ID) - if err != nil { - log.Error("GetOrgActionsConfig: %v", err) - ctx.HTTPError(http.StatusInternalServerError, "GetOrgActionsConfig", err.Error()) - return - } - if !cfg.AllowCrossRepoAccess { - ctx.HTTPError(http.StatusForbidden, "reqPackageAccess", "cross-repository package access is disabled") - return - } + if !cfg.AllowCrossRepoAccess { + ctx.HTTPError(http.StatusForbidden, "reqPackageAccess", "cross-repository package access is disabled") + return } - - // 3. Fallthrough to GetActionsUserRepoPermission - // We rely on the backend permission check below to handle other Cross-Repository restrictions - // (e.g., User collaborative owners, token scopes). } + + // 3. Fallthrough to GetActionsUserRepoPermission + // We rely on the backend permission check below to handle other Cross-Repository restrictions + // (e.g., User collaborative owners, token scopes). } } } diff --git a/routers/web/org/setting/actions.go b/routers/web/org/setting/actions.go index 4675df7919..949ceb1a2a 100644 --- a/routers/web/org/setting/actions.go +++ b/routers/web/org/setting/actions.go @@ -63,23 +63,26 @@ func ActionsGeneralPost(ctx *context.Context) { } if actionsCfg.TokenPermissionMode == repo_model.ActionsTokenPermissionModeCustom { + // Custom mode uses radio buttons for each permission scope parsePerm := func(name string) perm.AccessMode { - if ctx.FormBool(name + "_write") { + value := ctx.FormString(name) + switch value { + case "write": return perm.AccessModeWrite - } - if ctx.FormBool(name + "_read") { + case "read": return perm.AccessModeRead + default: + return perm.AccessModeNone } - return perm.AccessModeNone } actionsCfg.DefaultTokenPermissions = &repo_model.ActionsTokenPermissions{ - Actions: parsePerm("actions"), - Contents: parsePerm("contents"), - Issues: parsePerm("issues"), - Packages: parsePerm("packages"), - PullRequests: parsePerm("pull_requests"), - Wiki: parsePerm("wiki"), + Actions: parsePerm("perm_actions"), + Contents: parsePerm("perm_contents"), + Issues: parsePerm("perm_issues"), + Packages: parsePerm("perm_packages"), + PullRequests: parsePerm("perm_pull_requests"), + Wiki: parsePerm("perm_wiki"), } } else { actionsCfg.DefaultTokenPermissions = nil diff --git a/routers/web/repo/setting/actions.go b/routers/web/repo/setting/actions.go index a4f0c9488e..cd3b0ae7bf 100644 --- a/routers/web/repo/setting/actions.go +++ b/routers/web/repo/setting/actions.go @@ -156,23 +156,26 @@ func UpdateTokenPermissions(ctx *context.Context) { } if actionsCfg.TokenPermissionMode == repo_model.ActionsTokenPermissionModeCustom { + // Custom mode uses radio buttons for each permission scope parsePerm := func(name string) perm.AccessMode { - if ctx.FormBool(name + "_write") { + value := ctx.FormString(name) + switch value { + case "write": return perm.AccessModeWrite - } - if ctx.FormBool(name + "_read") { + case "read": return perm.AccessModeRead + default: + return perm.AccessModeNone } - return perm.AccessModeNone } actionsCfg.DefaultTokenPermissions = &repo_model.ActionsTokenPermissions{ - Actions: parsePerm("actions"), - Contents: parsePerm("contents"), - Issues: parsePerm("issues"), - Packages: parsePerm("packages"), - PullRequests: parsePerm("pull_requests"), - Wiki: parsePerm("wiki"), + Actions: parsePerm("perm_actions"), + Contents: parsePerm("perm_contents"), + Issues: parsePerm("perm_issues"), + Packages: parsePerm("perm_packages"), + PullRequests: parsePerm("perm_pull_requests"), + Wiki: parsePerm("perm_wiki"), } } else { actionsCfg.DefaultTokenPermissions = nil diff --git a/templates/org/settings/actions_general.tmpl b/templates/org/settings/actions_general.tmpl index fac5e9b40f..374e0cf46d 100644 --- a/templates/org/settings/actions_general.tmpl +++ b/templates/org/settings/actions_general.tmpl @@ -18,10 +18,32 @@
+ +
+ {{ctx.Locale.Tr "actions.general.token_permissions.mode"}} +
+
+
+
+ + +
+

{{ctx.Locale.Tr "actions.general.token_permissions.permissive.description"}}

+
+
+
+ + +
+

{{ctx.Locale.Tr "actions.general.token_permissions.restricted.description"}}

+
+
+ +
+
{{ctx.Locale.Tr "actions.general.token_permissions.maximum"}} - *

{{ctx.Locale.Tr "actions.general.token_permissions.maximum.description"}}

@@ -29,17 +51,16 @@ {{ctx.Locale.Tr "units.unit"}} - {{ctx.Locale.Tr "actions.general.token_permissions.access_none"}} ? - {{ctx.Locale.Tr "actions.general.token_permissions.access_read"}} ? - {{ctx.Locale.Tr "actions.general.token_permissions.access_write"}} ? + {{ctx.Locale.Tr "actions.general.token_permissions.access_none"}} + {{ctx.Locale.Tr "actions.general.token_permissions.access_read"}} + {{ctx.Locale.Tr "actions.general.token_permissions.access_write"}} - {{ctx.Locale.Tr "actions.general.token_permissions.contents"}} -

Access source code, files, commits and branches.

+

{{ctx.Locale.Tr "actions.general.token_permissions.contents.description"}}

@@ -60,11 +81,10 @@
- {{ctx.Locale.Tr "actions.general.token_permissions.issues"}} -

Organize bug reports, tasks and milestones.

+

{{ctx.Locale.Tr "actions.general.token_permissions.issues.description"}}

@@ -85,11 +105,10 @@
- {{ctx.Locale.Tr "actions.general.token_permissions.pull_requests"}} -

Enable pull requests and code reviews.

+

{{ctx.Locale.Tr "actions.general.token_permissions.pull_requests.description"}}

@@ -110,11 +129,10 @@
- {{ctx.Locale.Tr "actions.general.token_permissions.wiki"}} -

Write and share documentation with collaborators.

+

{{ctx.Locale.Tr "actions.general.token_permissions.wiki.description"}}

@@ -135,11 +153,10 @@
- {{ctx.Locale.Tr "actions.general.token_permissions.packages"}} -

Manage repository packages.

+

{{ctx.Locale.Tr "actions.general.token_permissions.packages.description"}}

@@ -160,11 +177,10 @@
- {{ctx.Locale.Tr "actions.general.token_permissions.actions_scope"}} -

Manage actions.

+

{{ctx.Locale.Tr "actions.general.token_permissions.actions_scope.description"}}

@@ -197,4 +213,3 @@
{{template "org/settings/layout_footer" .}} - diff --git a/templates/repo/settings/actions_general.tmpl b/templates/repo/settings/actions_general.tmpl index f6a2c0c9c2..35c3840001 100644 --- a/templates/repo/settings/actions_general.tmpl +++ b/templates/repo/settings/actions_general.tmpl @@ -53,7 +53,7 @@ {{ctx.Locale.Tr "actions.general.token_permissions.contents"}} -

Access source code, files, commits and branches.

+

{{ctx.Locale.Tr "actions.general.token_permissions.contents.description"}}

@@ -78,7 +78,7 @@ {{ctx.Locale.Tr "actions.general.token_permissions.issues"}} -

Organize bug reports, tasks and milestones.

+

{{ctx.Locale.Tr "actions.general.token_permissions.issues.description"}}

@@ -103,7 +103,7 @@ {{ctx.Locale.Tr "actions.general.token_permissions.pull_requests"}} -

Enable pull requests and code reviews.

+

{{ctx.Locale.Tr "actions.general.token_permissions.pull_requests.description"}}

@@ -128,7 +128,7 @@ {{ctx.Locale.Tr "actions.general.token_permissions.wiki"}} -

Write and share documentation with collaborators.

+

{{ctx.Locale.Tr "actions.general.token_permissions.wiki.description"}}

@@ -153,7 +153,7 @@ {{ctx.Locale.Tr "actions.general.token_permissions.packages"}} -

Manage repository packages.

+

{{ctx.Locale.Tr "actions.general.token_permissions.packages.description"}}

@@ -178,7 +178,7 @@ {{ctx.Locale.Tr "actions.general.token_permissions.actions_scope"}} -

Manage actions.

+

{{ctx.Locale.Tr "actions.general.token_permissions.actions_scope.description"}}

@@ -199,7 +199,7 @@
- +
diff --git a/tests/integration/actions_job_token_test.go b/tests/integration/actions_job_token_test.go index feac4fc8d5..61b18c6420 100644 --- a/tests/integration/actions_job_token_test.go +++ b/tests/integration/actions_job_token_test.go @@ -128,15 +128,17 @@ func TestActionsTokenPermissionsModes(t *testing.T) { func testActionsTokenPermissionsMode(u *url.URL, mode string, expectReadOnly bool) func(t *testing.T) { return func(t *testing.T) { - // Update repository settings to the requested mode + // Update repository settings to the requested mode if mode != "" { repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{Name: "repo4", OwnerName: "user5"}) require.NoError(t, repo.LoadUnits(t.Context())) - actionsUnit := repo.MustGetUnit(t.Context(), unit_model.TypeActions) + actionsUnit, err := repo.GetUnit(t.Context(), unit_model.TypeActions) + require.NoError(t, err, "Actions unit should exist for repo4") actionsCfg := actionsUnit.ActionsConfig() actionsCfg.TokenPermissionMode = repo_model.ActionsTokenPermissionMode(mode) actionsCfg.DefaultTokenPermissions = nil // Ensure no custom permissions override the mode actionsCfg.MaxTokenPermissions = nil // Ensure no max permissions interfere + // Update the config actionsUnit.Config = actionsCfg require.NoError(t, repo_model.UpdateRepoUnit(t.Context(), actionsUnit)) } diff --git a/web_src/js/features/repo-settings-actions.ts b/web_src/js/features/repo-settings-actions.ts index 3b0f93ba57..44809d2781 100644 --- a/web_src/js/features/repo-settings-actions.ts +++ b/web_src/js/features/repo-settings-actions.ts @@ -1,24 +1,6 @@ +// initRepoSettingsActionsPermissions is currently a no-op placeholder. +// The permission mode radio buttons work via standard HTML form submission +// without requiring JavaScript for toggling visibility. export function initRepoSettingsActionsPermissions(): void { - const radios = document.querySelectorAll( - 'input[name="token_permission_mode"]', - ); - if (!radios.length) return; - - function toggleCustom(): void { - const customPerms = document.querySelector('#custom-permissions'); - if (!customPerms) return; - - const selected = document.querySelector( - 'input[name="token_permission_mode"]:checked', - ); - - customPerms.style.display = - selected?.value === 'custom' ? 'block' : 'none'; - } - - for (const radio of radios) { - radio.addEventListener('change', toggleCustom); - } - - toggleCustom(); + // No-op - form submission handles permission updates }