Skip to content

fix: propagate max-ai-credits to external detector AWF config #50721

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 3 commits into from
Aug 6, 2026
+76 −0
Merged

fix: propagate max-ai-credits to external detector AWF config #50721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
461b1bd
Initial plan
Copilot Aug 6, 2026
bd3e549
fix: propagate MaxAICredits in buildExternalDetectorWorkflowData
Copilot Aug 6, 2026
d824057
Merge branch 'main' into copilot/threat-detection-propagate-max-ai-cr…
github-actions[bot] Aug 6, 2026
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
3 changes: 3 additions & 0 deletions pkg/workflow/threat_detection_helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,9 @@ func buildExternalDetectorWorkflowData(data *WorkflowData, engineID string) *Wor
if d.EngineConfig.APITarget == "" && data.EngineConfig != nil {
d.EngineConfig.APITarget = data.EngineConfig.APITarget
}
if data.SafeOutputs != nil && data.SafeOutputs.ThreatDetection != nil && data.SafeOutputs.ThreatDetection.MaxAICredits != 0 {

@github-actions github-actions Bot Aug 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/tdd] Missing interaction test: when canReuseThreatDetectionEngineConfigForExternalDetector returns true (i.e. ThreatDetection.EngineConfig != nil), the cloned engine config may already carry its own MaxAICredits. The new code unconditionally overwrites it with ThreatDetection.MaxAICredits, which is the correct priority rule, but there is no test covering both fields being set simultaneously.

💡 Suggested additional sub-test 
t.Run("ThreatDetection.MaxAICredits overrides EngineConfig.MaxAICredits when both set", func(t *testing.T) {
    data := &WorkflowData{
        AI: "copilot",
        SafeOutputs: &SafeOutputsConfig{
            ThreatDetection: &ThreatDetectionConfig{
                MaxAICredits: 777,
                EngineConfig: &EngineConfig{MaxAICredits: 999},
            },
        },
    }
    steps := compiler.buildExternalDetectorExecutionStep(data)
    allSteps := strings.Join(steps, "")
    if !strings.Contains(allSteps, `"maxAiCredits":777`) {
        t.Fatalf("expected top-level MaxAICredits 777 to win, got:\n%s", allSteps)
    }
})

This pins the priority rule: ThreatDetection.MaxAICredits wins over any value in ThreatDetection.EngineConfig.MaxAICredits.

@copilot please address this.

@github-actions github-actions Bot Aug 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: this relies on an implicit, unenforced invariant that ThreatDetection.EngineConfig.MaxAICredits is always zero for the external path — unlike the inline path, which explicitly rebuilds EngineConfig field-by-field to guarantee this.

💡 Details

buildExternalDetectorWorkflowData can populate d.EngineConfig via cloneThreatDetectionEngineConfig, which does a shallow *source copy of data.SafeOutputs.ThreatDetection.EngineConfig. Today that source is built through ExtractEngineConfig(map[string]any{"engine": engineObj}) — an isolated map with no top-level max-ai-credits key — so its MaxAICredits is always 0 in practice, and this new guarded assignment is safe.

But the sibling inline path (threat_detection_inline_engine.go) doesn't rely on that implicit invariant: it explicitly reconstructs a fresh EngineConfig{...} listing only the fields it wants to inherit and documents in a comment that MaxAICredits is intentionally omitted so it always starts at zero before applying the override. This diff's new block just adds a conditional overwrite to whatever d.EngineConfig.MaxAICredits shallow-copy already contains, without an explicit reset when ThreatDetection.MaxAICredits == 0.

Suggested hardening, mirroring the inline path's defensiveness:

if data.SafeOutputs != nil && data.SafeOutputs.ThreatDetection != nil {
    d.EngineConfig.MaxAICredits = data.SafeOutputs.ThreatDetection.MaxAICredits // always resets to 0 when unset
}

This removes the dependency on the current parsing behavior never populating that nested field, and a corresponding test (nonzero ThreatDetection.EngineConfig.MaxAICredits + zero ThreatDetection.MaxAICredits) would catch any future regression in ExtractEngineConfig's isolated-map parsing.

Not currently exploitable — no path today sets that nested field nonzero — so this is a defense-in-depth suggestion, not a blocker.

d.EngineConfig.MaxAICredits = data.SafeOutputs.ThreatDetection.MaxAICredits
}
return d
}

Expand Down
73 changes: 73 additions & 0 deletions pkg/workflow/threat_detection_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2945,3 +2945,76 @@ func TestBuildDetectionEngineExecutionStepPropagatesModelCostsProviders(t *testi
t.Errorf("expected detection awf-config.json to contain custom model pricing key; got:\n%s", allSteps)
}
}

func TestBuildExternalDetectorWorkflowDataMaxAICredits(t *testing.T) {
compiler := NewCompiler()

t.Run("uses detection runtime default expression when threat-detection max-ai-credits is unset", func(t *testing.T) {
data := &WorkflowData{
AI: "copilot",
SafeOutputs: &SafeOutputsConfig{
ThreatDetection: &ThreatDetectionConfig{},
},
}

steps := compiler.buildExternalDetectorExecutionStep(data)
allSteps := strings.Join(steps, "")
if !strings.Contains(allSteps, "vars."+compilerenv.DefaultDetectionMaxAICredits) {
t.Fatalf("expected external detector steps to reference vars.%s, got:\n%s", compilerenv.DefaultDetectionMaxAICredits, allSteps)
}
if !strings.Contains(allSteps, "'400'") {
t.Fatalf("expected external detector steps to include default fallback '400', got:\n%s", allSteps)
}
})

t.Run("uses explicit threat-detection max-ai-credits when provided", func(t *testing.T) {
data := &WorkflowData{
AI: "copilot",
SafeOutputs: &SafeOutputsConfig{
ThreatDetection: &ThreatDetectionConfig{
MaxAICredits: 777,
},
},
}

steps := compiler.buildExternalDetectorExecutionStep(data)
allSteps := strings.Join(steps, "")
if strings.Contains(allSteps, "vars."+compilerenv.DefaultDetectionMaxAICredits) {
t.Fatalf("expected external detector steps not to reference vars.%s when explicit max-ai-credits is set, got:\n%s", compilerenv.DefaultDetectionMaxAICredits, allSteps)
}
if !strings.Contains(allSteps, `"maxAiCredits":777`) {
t.Fatalf("expected external detector steps to include maxAiCredits 777, got:\n%s", allSteps)
}
})
}

func TestBuildExternalDetectorWorkflowDataMaxAICreditsNotInheritedFromMainAgent(t *testing.T) {
compiler := NewCompiler()

// When the main agent has an explicit MaxAICredits budget but
// safe-outputs.threat-detection.max-ai-credits is not set, the external
// detector must use its own runtime default expression rather than silently
// inheriting the agent budget.
data := &WorkflowData{
AI: "copilot",
EngineConfig: &EngineConfig{
MaxAICredits: 500, // explicit agent budget
},
SafeOutputs: &SafeOutputsConfig{
ThreatDetection: &ThreatDetectionConfig{
// max-ai-credits intentionally omitted
},
},
}

steps := compiler.buildExternalDetectorExecutionStep(data)
allSteps := strings.Join(steps, "")

if !strings.Contains(allSteps, "vars."+compilerenv.DefaultDetectionMaxAICredits) {
t.Fatalf("expected external detector steps to use runtime default expression vars.%s when detection max-ai-credits is unset, got:\n%s",
compilerenv.DefaultDetectionMaxAICredits, allSteps)
}
if strings.Contains(allSteps, `"maxAiCredits":500`) {
t.Fatalf("expected external detector steps NOT to inherit agent maxAiCredits=500, got:\n%s", allSteps)
}
}
Loading