Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions internal/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ type ApiClient interface {
GetUserMe() (string, error)
GetSelf() (contract.SelfResponse, error)
GetSastSettings(orgId string) (*sast_contract.SastResponse, error)
GetOrgSettings(orgId string) (*contract.OrgSettingsResponse, error)
}

var _ ApiClient = (*snykApiClient)(nil)
Expand Down Expand Up @@ -247,6 +248,21 @@ func (a *snykApiClient) GetSastSettings(orgId string) (*sast_contract.SastRespon
return &response, err
}

func (a *snykApiClient) GetOrgSettings(orgId string) (*contract.OrgSettingsResponse, error) {
endpoint := fmt.Sprintf("/v1/org/%s/settings", url.QueryEscape(orgId))
body, err := clientGet(a, endpoint, nil)
if err != nil {
return nil, fmt.Errorf("unable to retrieve org settings: %w", err)
}

var response contract.OrgSettingsResponse
if err = json.Unmarshal(body, &response); err != nil {
return nil, err
}

return &response, err
}

// clientGet performs an HTTP GET request to the Snyk API, handling query parameters,
// API versioning, and basic error checking.
//
Expand Down
16 changes: 16 additions & 0 deletions internal/api/contract/OrgSettingsResponse.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package contract

type OrgIgnoreSettings struct {
ReasonRequired bool `json:"reasonRequired,omitempty"`
AutoApproveIgnores bool `json:"autoApproveIgnores,omitempty"`
ApprovalWorkflowEnabled bool `json:"approvalWorkflowEnabled,omitempty"`
}

type OrgRequestAccessSettings struct {
Enabled bool `json:"enabled,omitempty"`
}

type OrgSettingsResponse struct {
Ignores *OrgIgnoreSettings `json:"ignores"`
RequestAccess *OrgRequestAccessSettings `json:"requestAccess"`
}
15 changes: 15 additions & 0 deletions internal/mocks/api.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 27 additions & 0 deletions pkg/local_workflows/ignore_workflow/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/google/uuid"

"github.com/snyk/error-catalog-golang-public/cli"
"github.com/snyk/go-application-framework/internal/api"
policyApi "github.com/snyk/go-application-framework/internal/api/policy/2024-10-15"
"github.com/snyk/go-application-framework/pkg/configuration"
"github.com/snyk/go-application-framework/pkg/local_workflows/local_models"
Expand Down Expand Up @@ -43,6 +44,32 @@ func addCreateIgnoreDefaultConfigurationValues(invocationCtx workflow.Invocation
})
}

func getOrgIgnoreApprovalEnabled(engine workflow.Engine) configuration.DefaultValueFunction {
return func(_ configuration.Configuration, existingValue interface{}) (interface{}, error) {
if existingValue != nil {
return existingValue, nil
}

config := engine.GetConfiguration()
org := config.GetString(configuration.ORGANIZATION)
client := engine.GetNetworkAccess().GetHttpClient()
url := config.GetString(configuration.API_URL)
apiClient := api.NewApi(url, client)

settings, err := apiClient.GetOrgSettings(org)
if err != nil {
engine.GetLogger().Err(err).Msg("Failed to access settings.")
return nil, err
}

if settings.Ignores == nil {
return false, nil
}

return settings.Ignores.ApprovalWorkflowEnabled, nil
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug

The getOrgIgnoreApprovalEnabled function incorrectly determines if the ignore approval workflow is enabled, leading to incorrect feature gating or workflow blocking, due to three issues:

  1. Incorrect Configuration Scope: It uses engine.GetConfiguration() and engine.GetNetworkAccess() instead of the invocation-specific configuration instance passed to its DefaultValueFunction. This can lead to querying settings for the wrong organization or using the wrong API endpoint/client when invocation overrides (e.g., --org) are present.
  2. Premature Defaulting for Booleans: It returns early if existingValue is not nil. For boolean configuration keys, an unset key often defaults to false (a non-nil zero value), which bypasses the actual settings fetch and forces the feature to be false by default, incorrectly disabling it.
  3. Swallowed Errors: If fetching settings fails, it returns (nil, err). Since config.GetBool() (the likely caller) swallows this error, the value effectively defaults to false, leading to the workflow being blocked as if the feature were disabled, even due to transient network or backend issues.
Fix in Cursor Fix in Web


func remoteRepoUrlDefaultFunc(existingValue interface{}, config configuration.Configuration) (interface{}, error) {
if existingValue != nil && existingValue != "" {
return existingValue, nil
Expand Down
23 changes: 18 additions & 5 deletions pkg/local_workflows/ignore_workflow/ignore_workflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ import (

"github.com/snyk/code-client-go/sarif"
"github.com/snyk/error-catalog-golang-public/cli"
"github.com/snyk/error-catalog-golang-public/snyk_errors"

policyApi "github.com/snyk/go-application-framework/internal/api/policy/2024-10-15"
"github.com/snyk/go-application-framework/pkg/configuration"
"github.com/snyk/go-application-framework/pkg/local_workflows"
"github.com/snyk/go-application-framework/pkg/local_workflows/config_utils"
localworkflows "github.com/snyk/go-application-framework/pkg/local_workflows"
"github.com/snyk/go-application-framework/pkg/local_workflows/local_models"
"github.com/snyk/go-application-framework/pkg/utils/git"
"github.com/snyk/go-application-framework/pkg/workflow"
Expand Down Expand Up @@ -60,6 +60,8 @@ const (

interactiveEnsureVersionControlMessage = "👉🏼 Ensure the code containing the issue is committed and pushed to remote origin, so approvers can review it."
interactiveIgnoreRequestSubmissionMessage = "✅ Your ignore request has been submitted."

ConfigIgnoreApprovalEnabled = "internal_iaw_enabled"
)

var reasonPromptHelpMap = map[string]string{
Expand Down Expand Up @@ -88,7 +90,7 @@ func InitIgnoreWorkflows(engine workflow.Engine) error {
return err
}

config_utils.AddFeatureFlagToConfig(engine, configuration.FF_IAW_ENABLED, "ignoreApprovalWorkflow")
engine.GetConfiguration().AddDefaultValue(ConfigIgnoreApprovalEnabled, getOrgIgnoreApprovalEnabled(engine))

return nil
}
Expand All @@ -100,8 +102,19 @@ func ignoreCreateWorkflowEntryPoint(invocationCtx workflow.InvocationContext, _
config := invocationCtx.GetConfiguration()
id := invocationCtx.GetWorkflowIdentifier()

if !config.GetBool(configuration.FF_IAW_ENABLED) {
return nil, cli.NewFeatureUnderDevelopmentError("")
if !config.GetBool(ConfigIgnoreApprovalEnabled) {
orgName := config.GetString(configuration.ORGANIZATION_SLUG)
return nil, snyk_errors.Error{
Type: "https://docs.snyk.io/scan-with-snyk/error-catalog#snyk-cli-0016",
Title: "Feature not enabled",
Description: "This feature is disabled for your current organization. You can enable it in the settings or switch to an organization where it's already enabled.",
StatusCode: 403,
ErrorCode: "SNYK-CLI-0016",
Classification: "ACTIONABLE",
Links: []string{},
Level: "error",
Detail: fmt.Sprintf("The Ignore Approval Workflow feature must be enabled for the %s organization. Enable it in your organization settings: Settings > General > Ignore approval workflow for Snyk Code.", orgName),
}
}

interactive := config.GetBool(InteractiveKey)
Expand Down
6 changes: 3 additions & 3 deletions pkg/local_workflows/ignore_workflow/ignore_workflow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func setupMockIgnoreContext(t *testing.T, payload string, statusCode int) *mocks
config := configuration.New()
config.Set(configuration.API_URL, "https://api.snyk.io")
config.Set(configuration.ORGANIZATION, uuid.New().String())
config.Set(configuration.FF_IAW_ENABLED, true)
config.Set(ConfigIgnoreApprovalEnabled, true)
// setup mocks
ctrl := gomock.NewController(t)
networkAccessMock := mocks.NewMockNetworkAccess(ctrl)
Expand Down Expand Up @@ -306,7 +306,7 @@ func Test_ignoreCreateWorkflowEntryPoint(t *testing.T) {

invocationContext := setupMockIgnoreContext(t, "{}", http.StatusCreated)
config := invocationContext.GetConfiguration()
config.Set(configuration.FF_IAW_ENABLED, false)
config.Set(ConfigIgnoreApprovalEnabled, false)

config.Set(InteractiveKey, false)

Expand All @@ -330,7 +330,7 @@ func setupInteractiveMockContext(t *testing.T, mockApiResponse string, mockApiSt
config := configuration.New()
config.Set(configuration.API_URL, "https://api.snyk.io")
config.Set(configuration.ORGANIZATION, uuid.New().String())
config.Set(configuration.FF_IAW_ENABLED, true)
config.Set(ConfigIgnoreApprovalEnabled, true)
config.Set(InteractiveKey, true) // Always interactive
config.Set(EnrichResponseKey, true)
config.Set(configuration.ORGANIZATION_SLUG, "some-org")
Expand Down