From d713f3a26ef59065ea2cdd0ce54e3baff3e71501 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 22 May 2026 15:05:54 -0700 Subject: [PATCH] update --- models/issues/comment_meta_test.go | 8 +- routers/api/v1/repo/project_workflow.go | 60 +++++++---- routers/web/projects/workflows.go | 136 ++++++++++++++---------- services/issue/label.go | 14 +++ services/projects/workflow_notifier.go | 12 ++- 5 files changed, 146 insertions(+), 84 deletions(-) diff --git a/models/issues/comment_meta_test.go b/models/issues/comment_meta_test.go index c3ee92e8a0..59e139382d 100644 --- a/models/issues/comment_meta_test.go +++ b/models/issues/comment_meta_test.go @@ -40,12 +40,8 @@ func TestBuildCreateCommentMetaData(t *testing.T) { assert.Equal(t, SpecialDoerNameCodeOwners, meta.SpecialDoerName) assert.Zero(t, meta.ProjectWorkflowID) - // NewProjectWorkflowDoer correctly populates workflow metadata. - // This is the primary regression guard: the type assertion at - // buildCreateCommentMetaData line 870 must be *projectWorkflowDoer (pointer). - // If it were changed to projectWorkflowDoer (value), opts.Doer.ExtDoerData - // (which is *projectWorkflowDoer) would not match, ok==false, and all - // workflow fields would silently remain zero — caught here. + // ExtDoerData must be stored as *projectWorkflowDoer (pointer); a value type + // would not match the type assertion and all workflow fields would silently be zero. const ( wfID = int64(42) wfEvent = project_model.WorkflowEventItemOpened diff --git a/routers/api/v1/repo/project_workflow.go b/routers/api/v1/repo/project_workflow.go index 57007d5e1f..7742ee4d34 100644 --- a/routers/api/v1/repo/project_workflow.go +++ b/routers/api/v1/repo/project_workflow.go @@ -118,9 +118,15 @@ func listAPIProjectWorkflows(ctx *api_context.APIContext, project *project_model return result, nil } -func convertAPIProjectWorkflowFilters(ctx *api_context.APIContext, project *project_model.Project, options api.ProjectWorkflowFilterOptions) []project_model.WorkflowFilter { +func convertAPIProjectWorkflowFilters(ctx *api_context.APIContext, project *project_model.Project, event project_model.WorkflowEvent, options api.ProjectWorkflowFilterOptions) []project_model.WorkflowFilter { + caps := project_model.GetWorkflowEventCapabilities()[event] + allowedFilters := make(map[project_model.WorkflowFilterType]bool, len(caps.AvailableFilters)) + for _, ft := range caps.AvailableFilters { + allowedFilters[ft] = true + } + filters := make([]project_model.WorkflowFilter, 0) - if options.IssueType != "" { + if allowedFilters[project_model.WorkflowFilterTypeIssueType] && options.IssueType != "" { filters = append(filters, project_model.WorkflowFilter{Type: project_model.WorkflowFilterTypeIssueType, Value: options.IssueType}) } @@ -131,7 +137,7 @@ func convertAPIProjectWorkflowFilters(ctx *api_context.APIContext, project *proj {typeName: project_model.WorkflowFilterTypeSourceColumn, value: options.SourceColumn}, {typeName: project_model.WorkflowFilterTypeTargetColumn, value: options.TargetColumn}, } { - if item.value == "" { + if !allowedFilters[item.typeName] || item.value == "" { continue } columnID, _ := strconv.ParseInt(item.value, 10, 64) @@ -145,22 +151,30 @@ func convertAPIProjectWorkflowFilters(ctx *api_context.APIContext, project *proj filters = append(filters, project_model.WorkflowFilter{Type: item.typeName, Value: strconv.FormatInt(columnID, 10)}) } - for _, label := range options.Labels { - if label == "" { - continue - } - labelID, _ := strconv.ParseInt(label, 10, 64) - if project_service.CanProjectAddLabel(ctx, project, labelID) { - filters = append(filters, project_model.WorkflowFilter{Type: project_model.WorkflowFilterTypeLabels, Value: label}) + if allowedFilters[project_model.WorkflowFilterTypeLabels] { + for _, label := range options.Labels { + if label == "" { + continue + } + labelID, _ := strconv.ParseInt(label, 10, 64) + if project_service.CanProjectAddLabel(ctx, project, labelID) { + filters = append(filters, project_model.WorkflowFilter{Type: project_model.WorkflowFilterTypeLabels, Value: label}) + } } } return filters } -func convertAPIProjectWorkflowActions(ctx *api_context.APIContext, project *project_model.Project, options api.ProjectWorkflowActionOptions) []project_model.WorkflowAction { +func convertAPIProjectWorkflowActions(ctx *api_context.APIContext, project *project_model.Project, event project_model.WorkflowEvent, options api.ProjectWorkflowActionOptions) []project_model.WorkflowAction { + caps := project_model.GetWorkflowEventCapabilities()[event] + allowedActions := make(map[project_model.WorkflowActionType]bool, len(caps.AvailableActions)) + for _, at := range caps.AvailableActions { + allowedActions[at] = true + } + actions := make([]project_model.WorkflowAction, 0) - if options.Column != "" { + if allowedActions[project_model.WorkflowActionTypeColumn] && options.Column != "" { columnID, _ := strconv.ParseInt(options.Column, 10, 64) if columnID > 0 { column, _ := project_model.GetColumnByIDAndProjectID(ctx, columnID, project.ID) @@ -177,6 +191,9 @@ func convertAPIProjectWorkflowActions(ctx *api_context.APIContext, project *proj {typeName: project_model.WorkflowActionTypeAddLabels, labels: options.AddLabels}, {typeName: project_model.WorkflowActionTypeRemoveLabels, labels: options.RemoveLabels}, } { + if !allowedActions[entry.typeName] { + continue + } for _, label := range entry.labels { if label == "" { continue @@ -188,9 +205,11 @@ func convertAPIProjectWorkflowActions(ctx *api_context.APIContext, project *proj } } - issueState := strings.ToLower(options.IssueState) - if issueState == "close" || issueState == "reopen" { - actions = append(actions, project_model.WorkflowAction{Type: project_model.WorkflowActionTypeIssueState, Value: issueState}) + if allowedActions[project_model.WorkflowActionTypeIssueState] { + issueState := strings.ToLower(options.IssueState) + if issueState == "close" || issueState == "reopen" { + actions = append(actions, project_model.WorkflowAction{Type: project_model.WorkflowActionTypeIssueState, Value: issueState}) + } } return actions @@ -432,11 +451,12 @@ func CreateProjectWorkflow(ctx *api_context.APIContext) { ctx.APIError(http.StatusUnprocessableEntity, "invalid event_id: "+form.EventID) return } + workflowEvent := project_model.WorkflowEvent(form.EventID) workflow := &project_model.Workflow{ ProjectID: project.ID, - WorkflowEvent: project_model.WorkflowEvent(form.EventID), - WorkflowFilters: convertAPIProjectWorkflowFilters(ctx, project, form.Filters), - WorkflowActions: convertAPIProjectWorkflowActions(ctx, project, form.Actions), + WorkflowEvent: workflowEvent, + WorkflowFilters: convertAPIProjectWorkflowFilters(ctx, project, workflowEvent, form.Filters), + WorkflowActions: convertAPIProjectWorkflowActions(ctx, project, workflowEvent, form.Actions), Enabled: true, } if len(workflow.WorkflowActions) == 0 { @@ -537,8 +557,8 @@ func UpdateProjectWorkflow(ctx *api_context.APIContext) { return } form := web.GetForm(ctx).(*api.EditProjectWorkflowOption) - workflow.WorkflowFilters = convertAPIProjectWorkflowFilters(ctx, project, form.Filters) - workflow.WorkflowActions = convertAPIProjectWorkflowActions(ctx, project, form.Actions) + workflow.WorkflowFilters = convertAPIProjectWorkflowFilters(ctx, project, workflow.WorkflowEvent, form.Filters) + workflow.WorkflowActions = convertAPIProjectWorkflowActions(ctx, project, workflow.WorkflowEvent, form.Actions) if len(workflow.WorkflowActions) == 0 { ctx.APIError(http.StatusUnprocessableEntity, errors.New("at least one action is required")) return diff --git a/routers/web/projects/workflows.go b/routers/web/projects/workflows.go index 73f8b10146..bb35f6eb62 100644 --- a/routers/web/projects/workflows.go +++ b/routers/web/projects/workflows.go @@ -27,12 +27,22 @@ var ( ) // convertFormToFilters converts form filters to WorkflowFilter objects -func convertFormToFilters(ctx stdCtx.Context, project *project_model.Project, formFilters map[string]any) []project_model.WorkflowFilter { +func convertFormToFilters(ctx stdCtx.Context, project *project_model.Project, event project_model.WorkflowEvent, formFilters map[string]any) []project_model.WorkflowFilter { filters := make([]project_model.WorkflowFilter, 0) + caps := project_model.GetWorkflowEventCapabilities()[event] + allowed := make(map[project_model.WorkflowFilterType]bool, len(caps.AvailableFilters)) + for _, ft := range caps.AvailableFilters { + allowed[ft] = true + } + for key, value := range formFilters { - switch key { - case string(project_model.WorkflowFilterTypeLabels): + filterType := project_model.WorkflowFilterType(key) + if !allowed[filterType] { + continue // not supported for this event + } + switch filterType { + case project_model.WorkflowFilterTypeLabels: // Handle labels array if labelInterfaces, ok := value.([]any); ok && len(labelInterfaces) > 0 { for _, labelInterface := range labelInterfaces { @@ -40,14 +50,14 @@ func convertFormToFilters(ctx stdCtx.Context, project *project_model.Project, fo labelID, _ := strconv.ParseInt(label, 10, 64) if project_service.CanProjectAddLabel(ctx, project, labelID) { filters = append(filters, project_model.WorkflowFilter{ - Type: project_model.WorkflowFilterTypeLabels, + Type: filterType, Value: label, }) } } } } - case string(project_model.WorkflowFilterTypeSourceColumn), string(project_model.WorkflowFilterTypeTargetColumn): + case project_model.WorkflowFilterTypeSourceColumn, project_model.WorkflowFilterTypeTargetColumn: if strValue, ok := value.(string); ok && strValue != "" { strValueInt, _ := strconv.ParseInt(strValue, 10, 64) if strValueInt > 0 { @@ -56,7 +66,7 @@ func convertFormToFilters(ctx stdCtx.Context, project *project_model.Project, fo continue } filters = append(filters, project_model.WorkflowFilter{ - Type: project_model.WorkflowFilterType(key), + Type: filterType, Value: strconv.FormatInt(strValueInt, 10), }) } @@ -65,7 +75,7 @@ func convertFormToFilters(ctx stdCtx.Context, project *project_model.Project, fo // Handle string values (issue_type, column) if strValue, ok := value.(string); ok && strValue != "" { filters = append(filters, project_model.WorkflowFilter{ - Type: project_model.WorkflowFilterType(key), + Type: filterType, Value: strValue, }) } @@ -76,12 +86,22 @@ func convertFormToFilters(ctx stdCtx.Context, project *project_model.Project, fo } // convertFormToActions converts form actions to WorkflowAction objects -func convertFormToActions(ctx stdCtx.Context, project *project_model.Project, formActions map[string]any) []project_model.WorkflowAction { +func convertFormToActions(ctx stdCtx.Context, project *project_model.Project, event project_model.WorkflowEvent, formActions map[string]any) []project_model.WorkflowAction { actions := make([]project_model.WorkflowAction, 0) + caps := project_model.GetWorkflowEventCapabilities()[event] + allowed := make(map[project_model.WorkflowActionType]bool, len(caps.AvailableActions)) + for _, at := range caps.AvailableActions { + allowed[at] = true + } + for key, value := range formActions { - switch key { - case string(project_model.WorkflowActionTypeColumn): + actionType := project_model.WorkflowActionType(key) + if !allowed[actionType] { + continue // not supported for this event + } + switch actionType { + case project_model.WorkflowActionTypeColumn: if colValue, ok := value.(string); ok { colValueInt, _ := strconv.ParseInt(colValue, 10, 64) if colValueInt > 0 { @@ -95,7 +115,7 @@ func convertFormToActions(ctx stdCtx.Context, project *project_model.Project, fo }) } } - case string(project_model.WorkflowActionTypeAddLabels): + case project_model.WorkflowActionTypeAddLabels: // Handle both []string and []any from JSON unmarshaling if labels, ok := value.([]string); ok && len(labels) > 0 { for _, label := range labels { @@ -123,7 +143,7 @@ func convertFormToActions(ctx stdCtx.Context, project *project_model.Project, fo } } } - case string(project_model.WorkflowActionTypeRemoveLabels): + case project_model.WorkflowActionTypeRemoveLabels: // Handle both []string and []any from JSON unmarshaling if labels, ok := value.([]string); ok && len(labels) > 0 { for _, label := range labels { @@ -152,7 +172,7 @@ func convertFormToActions(ctx stdCtx.Context, project *project_model.Project, fo } } } - case string(project_model.WorkflowActionTypeIssueState): + case project_model.WorkflowActionTypeIssueState: if strValue, ok := value.(string); ok { v := strings.ToLower(strValue) if v == "close" || v == "reopen" { @@ -397,8 +417,6 @@ func WorkflowsPost(ctx *context.Context) { return } - // Handle both form data and JSON data - // Handle JSON data form := &WorkflowsPostForm{} content, err := io.ReadAll(ctx.Req.Body) if err != nil { @@ -415,38 +433,54 @@ func WorkflowsPost(ctx *context.Context) { return } - // Convert form data to filters and actions - filters := convertFormToFilters(ctx, p, form.Filters) - actions := convertFormToActions(ctx, p, form.Actions) + // Determine the workflow event before converting filters/actions so we can + // validate against the event's capabilities. + eventID, _ := strconv.ParseInt(form.EventID, 10, 64) + var ( + workflowEvent project_model.WorkflowEvent + existingWf *project_model.Workflow + ) + if eventID == 0 { + if !project_model.IsValidWorkflowEvent(form.EventID) { + ctx.JSONError(fmt.Sprintf("EventID %s is invalid", form.EventID)) + return + } + workflowEvent = project_model.WorkflowEvent(form.EventID) + } else { + existingWf, err = project_model.GetWorkflowByProjectAndID(ctx, p.ID, eventID) + if err != nil { + if db.IsErrNotExist(err) { + ctx.NotFound(nil) + } else { + ctx.ServerError("GetWorkflowByID", err) + } + return + } + workflowEvent = existingWf.WorkflowEvent + } + + // Convert and validate filters/actions against the event's capabilities. + filters := convertFormToFilters(ctx, p, workflowEvent, form.Filters) + actions := convertFormToActions(ctx, p, workflowEvent, form.Actions) - // Validate: at least one action must be configured if len(actions) == 0 { ctx.JSONError(ctx.Tr("projects.workflows.at_least_one_action_required")) return } - eventID, _ := strconv.ParseInt(form.EventID, 10, 64) - if eventID == 0 { - // if it's not a real database id, check if it's the event string - if !project_model.IsValidWorkflowEvent(form.EventID) { - ctx.JSONError(fmt.Sprintf("EventID %s is invalid", form.EventID)) - return - } - - // Create a new workflow for the given event + if existingWf == nil { + // Create a new workflow for the given event. wf := &project_model.Workflow{ ProjectID: p.ID, - WorkflowEvent: project_model.WorkflowEvent(form.EventID), + WorkflowEvent: workflowEvent, WorkflowFilters: filters, WorkflowActions: actions, - Enabled: true, // New workflows are enabled by default + Enabled: true, } if err := project_model.CreateWorkflow(ctx, wf); err != nil { ctx.ServerError("CreateWorkflow", err) return } - - // Return the newly created workflow with filter summary workflowSummary := project_service.GetWorkflowSummary(ctx, wf) ctx.JSON(http.StatusOK, map[string]any{ "success": true, @@ -466,38 +500,26 @@ func WorkflowsPost(ctx *context.Context) { return } - // Update an existing workflow - wf, err := project_model.GetWorkflowByProjectAndID(ctx, p.ID, eventID) - if err != nil { - if db.IsErrNotExist(err) { - ctx.NotFound(nil) - } else { - ctx.ServerError("GetWorkflowByID", err) - } - return - } - - wf.WorkflowFilters = filters - wf.WorkflowActions = actions - if err := project_model.UpdateWorkflow(ctx, wf); err != nil { + // Update the existing workflow. + existingWf.WorkflowFilters = filters + existingWf.WorkflowActions = actions + if err := project_model.UpdateWorkflow(ctx, existingWf); err != nil { ctx.ServerError("UpdateWorkflow", err) return } - - // Return the updated workflow with filter summary - workflowSummary := project_service.GetWorkflowSummary(ctx, wf) + workflowSummary := project_service.GetWorkflowSummary(ctx, existingWf) ctx.JSON(http.StatusOK, map[string]any{ "success": true, "workflow": WorkflowConfig{ - ID: wf.ID, - EventID: strconv.FormatInt(wf.ID, 10), - DisplayName: string(ctx.Tr(wf.WorkflowEvent.LangKey())), - WorkflowEvent: string(wf.WorkflowEvent), - Capabilities: project_model.GetWorkflowEventCapabilities()[wf.WorkflowEvent], - Filters: wf.WorkflowFilters, - Actions: wf.WorkflowActions, + ID: existingWf.ID, + EventID: strconv.FormatInt(existingWf.ID, 10), + DisplayName: string(ctx.Tr(existingWf.WorkflowEvent.LangKey())), + WorkflowEvent: string(existingWf.WorkflowEvent), + Capabilities: project_model.GetWorkflowEventCapabilities()[existingWf.WorkflowEvent], + Filters: existingWf.WorkflowFilters, + Actions: existingWf.WorkflowActions, Summary: workflowSummary, - Enabled: wf.Enabled, + Enabled: existingWf.Enabled, IsConfigured: true, }, }) diff --git a/services/issue/label.go b/services/issue/label.go index 77bef8db1a..b28f3e1f0c 100644 --- a/services/issue/label.go +++ b/services/issue/label.go @@ -8,6 +8,7 @@ import ( "code.gitea.io/gitea/models/db" issues_model "code.gitea.io/gitea/models/issues" + access_model "code.gitea.io/gitea/models/perm/access" user_model "code.gitea.io/gitea/models/user" notify_service "code.gitea.io/gitea/services/notify" ) @@ -46,6 +47,19 @@ func AddLabels(ctx context.Context, issue *issues_model.Issue, doer *user_model. // RemoveLabel removes a label from issue by given ID. func RemoveLabel(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, label *issues_model.Label) error { if err := db.WithTx(ctx, func(ctx context.Context) error { + if err := issue.LoadRepo(ctx); err != nil { + return err + } + perm, err := access_model.GetDoerRepoPermission(ctx, issue.Repo, doer) + if err != nil { + return err + } + if !perm.CanWriteIssuesOrPulls(issue.IsPull) { + if label.OrgID > 0 { + return issues_model.ErrOrgLabelNotExist{} + } + return issues_model.ErrRepoLabelNotExist{} + } return issues_model.DeleteIssueLabel(ctx, issue, label, doer) }); err != nil { return err diff --git a/services/projects/workflow_notifier.go b/services/projects/workflow_notifier.go index 5705c4a400..db2e8face5 100644 --- a/services/projects/workflow_notifier.go +++ b/services/projects/workflow_notifier.go @@ -71,6 +71,11 @@ func (m *workflowNotifier) NewPullRequest(ctx context.Context, pr *issues_model. } func (m *workflowNotifier) IssueChangeStatus(ctx context.Context, doer *user_model.User, commitID string, issue *issues_model.Issue, actionComment *issues_model.Comment, isClosed bool) { + // Skip state changes triggered by workflow actions to prevent cascade loops + // (same guard as feed/notifier.go). + if doer.ExtDoerData != nil { + return + } if err := issue.LoadRepo(ctx); err != nil { log.Error("IssueChangeStatus: LoadRepo: %v", err) return @@ -103,7 +108,8 @@ func (m *workflowNotifier) IssueChangeStatus(ctx context.Context, doer *user_mod func (*workflowNotifier) IssueChangeProjects(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, oldProjectColumnMap map[int64]int64, newProjects []*project_model.Project) { addedProjects := make(map[int64]*project_model.Project) for _, newProject := range newProjects { - if oldProjectColumnMap[newProject.ID] != 0 { + // Use key presence check; column ID 0 is technically valid. + if _, ok := oldProjectColumnMap[newProject.ID]; ok { continue } addedProjects[newProject.ID] = newProject @@ -159,6 +165,10 @@ func (*workflowNotifier) IssueChangeProjects(ctx context.Context, doer *user_mod } func (*workflowNotifier) IssueChangeProjectColumn(ctx context.Context, doer *user_model.User, issue *issues_model.Issue, oldColumnID, newColumnID int64) { + // Skip column moves triggered by workflow actions to prevent cascade loops. + if doer.ExtDoerData != nil { + return + } if err := issue.LoadRepo(ctx); err != nil { log.Error("IssueChangeStatus: LoadRepo: %v", err) return