Skip to content

fix(aggregate): replace max-only scoring with additive aggregation to prevent multi-signal bypass#46

Open
RachanaB5 wants to merge 1 commit into
c2siorg:mainfrom
RachanaB5:fix/aggregate-max-only-scoring-ignores-multiple-signals
Open

fix(aggregate): replace max-only scoring with additive aggregation to prevent multi-signal bypass#46
RachanaB5 wants to merge 1 commit into
c2siorg:mainfrom
RachanaB5:fix/aggregate-max-only-scoring-ignores-multiple-signals

Conversation

@RachanaB5

@RachanaB5 RachanaB5 commented Apr 19, 2026

Copy link
Copy Markdown
Contributor

Problem

The current aggregation logic uses a max-only strategy:

func maxSignalScore(signals []riskcontext.Signal) float64 {
    var max float64
    for _, sig := range signals {
        if sig.Score > max {
            max = sig.Score
        }
    }
    return max
}

This causes only the highest signal to influence the final score, while all other signals are ignored.

As a result, multiple medium-risk signals do not combine to reflect cumulative risk, allowing threshold-based decisions to be bypassed.

Proof of Bug (before fix)

Two signals each scoring 0.45:

Expected: 0.45 + 0.45 = 0.90 → BLOCK (≥ 0.85)
Actual:   max(0.45, 0.45) = 0.45 → ALLOW

Test failure:

--- FAIL: TestPipeline_MultipleSignalsCombine
    pipeline_test.go:148: expected BLOCK when two signals combine above threshold,
        got decision=0 score=0.45

This confirms:

  • Score computed as 0.45 (max) instead of 0.90 (sum)
  • Incorrect decision: ALLOW instead of BLOCK

Fix

Replace max-only aggregation with additive scoring:

func sumSignalScore(signals []riskcontext.Signal) float64 {
    var total float64
    for _, sig := range signals {
        total += sig.Score
    }
    return total
}

The final score is already clamped to [0.0, 1.0] in AggregateStage.Run, so no additional bounds checks are required.

Proof of Fix (after change)

    pipeline_test.go:147: signal[0]=0.45 + signal[1]=0.45 → score=0.90 → decision=2 (0=ALLOW 1=SANITISE 2=BLOCK)
=== RUN   TestPipeline_MultipleSignalsCombine
--- PASS: TestPipeline_MultipleSignalsCombine (0.00s)

Full test suite:

ok  github.com/acf-sdk/sidecar/internal/config
ok  github.com/acf-sdk/sidecar/internal/crypto
ok  github.com/acf-sdk/sidecar/internal/pipeline
ok  github.com/acf-sdk/sidecar/internal/transport

✅ All 49 tests passing — no regressions

Changes

  • internal/pipeline/aggregate.go — replace maxSignalScore with sumSignalScore
  • internal/pipeline/pipeline_test.go — add TestPipeline_MultipleSignalsCombine to validate cumulative scoring

Impact

  • Fixes incorrect risk aggregation logic
  • Prevents multi-signal bypass scenarios
  • Ensures decisions reflect cumulative risk instead of a single dominant signal

closes #45

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] fix(aggregate): incorrect max-only scoring ignores multiple signals

1 participant