Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
10 changes: 10 additions & 0 deletions docs/changes/unreleased/1815-security-affinity-accounting.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
kind: fixed
title: security audits distinguish incomplete analysis from a completed search
pr: 1815
surface: [chat, engine]
invalidates:
- "Unicode session labels were also sent as machine affinity keys, which some model gateways rejected. Unsafe keys are now stable ASCII while the readable labels and selected models remain unchanged."
- "Failed hunt calls contributed empty batches without measured failure accounting, so every failed hunt could end as a clean pass. Failed attempts now survive checkpoints and reports, make the analysis incomplete and retain findings from completed hunts."
- "A partial audit result could conceal a later model refusal or run limit. Result-bearing stops now keep their failed or ceiling ending and mark Markdown, JSON and SARIF as incomplete without changing proof requirements."
---
46 changes: 46 additions & 0 deletions internal/agentsession/affinity_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package agentsession

import (
"strings"
"testing"
)

// DISPLAY WORDS STAY WORDS, but a gateway may reuse the cache key as a request
// header. Exercise the actual wire body across turns, labels and sessions.
func TestUnicodeLabelsKeepTheirWordsAndUseStableASCIIWireAffinity(t *testing.T) {
root := repo(t)
label := "auth hunter · src/登录.py:12"
api := (&fakeAPI{}).then(callTool("read", `{"path":"app/views.py"}`)).
then(answer(`{"file":"app/views.py","line":4}`, 0)).
then(answer(`{"file":"app/views.py","line":4}`, 0)).
then(answer(`{"file":"app/views.py","line":4}`, 0))
var heard []string
app := mustNew(t, api.serve(t), Config{Root: root, SessionModel: "vendor/work",
Watch: &Watch{Session: func(label string, _ SessionResult, _ error) { heard = append(heard, label) }}})
for _, name := range []string{label, label, "auth hunter · src/登入.py:12"} {
opts := appxOptions("", root)
opts.Label = name
var dest map[string]any
result, err := app.Harness(t.Context(), "read and answer", findingSchema, &dest, opts)
if err != nil || result.IsError {
t.Fatalf("session %q: %+v, %v", name, result, err)
}
}
if strings.Join(heard, "|") != label+"|"+label+"|auth hunter · src/登入.py:12" {
t.Fatalf("display labels changed: %v", heard)
}
for _, body := range api.bodies {
if body.PromptCacheKey == "" || strings.IndexFunc(body.PromptCacheKey, func(r rune) bool { return r < 33 || r > 126 }) >= 0 {
t.Fatalf("unsafe wire affinity %q", body.PromptCacheKey)
}
if body.Model != "vendor/work" || len(body.Messages) == 0 || len(body.Tools) == 0 {
t.Fatalf("model, messages or tools lost: %+v", body)
}
}
if api.bodies[0].PromptCacheKey != api.bodies[1].PromptCacheKey {
t.Fatal("a tool turn changed the session's affinity")
}
if api.bodies[1].PromptCacheKey == api.bodies[2].PromptCacheKey || api.bodies[2].PromptCacheKey == api.bodies[3].PromptCacheKey {
t.Fatal("distinct sessions or Unicode labels share affinity")
}
}
25 changes: 24 additions & 1 deletion internal/agentsession/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ package agentsession
import (
"bytes"
"context"
"crypto/sha256"
"encoding/json"
"errors"
"fmt"
Expand Down Expand Up @@ -141,7 +142,7 @@ func (c *Client) Complete(ctx context.Context, request Request) (*ai.Response, e
}
wire := wireRequest{
Model: request.Model, Messages: request.Messages, Tools: request.Tools,
ResponseFormat: request.ResponseFormat, PromptCacheKey: request.Thread,
ResponseFormat: request.ResponseFormat, PromptCacheKey: wireAffinity(request.Thread),
}
if len(wire.Tools) > 0 {
wire.ToolChoice = "auto"
Expand Down Expand Up @@ -189,6 +190,28 @@ func (c *Client) Complete(ctx context.Context, request Request) (*ai.Response, e
return nil, last
}

// wireAffinity keeps ordinary cache keys unchanged. A display label can contain
// any Unicode, but gateways also use affinity as a request header, which must
// be ASCII. Hash the entire unsafe key so its turns stay together without
// collapsing different labels or session numbers to the same stripped text.
func wireAffinity(thread string) string {
if strings.IndexFunc(thread, func(r rune) bool { return r < 33 || r > 126 }) < 0 {
return thread
}
sum := sha256.Sum256([]byte(thread))
return fmt.Sprintf("agent-%x", sum)
}

// Refusal is the first refusal that stopped all model calls. A program that
// retains a partial result still needs the typed ceiling or key refusal, even
// if its own phase calls wrapped the individual failures in opaque errors.
func (c *Client) Refusal() error {
if stopped := c.stopped.Load(); stopped != nil {
return *stopped
}
return nil
}

// errHeld is the model API asking a call to wait while calls in flight hold
// what is left of the run's ceiling.
var errHeld = errors.New("the run's ceiling is held by calls in flight")
Expand Down
19 changes: 19 additions & 0 deletions internal/manual/chat/sec.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,25 @@ The full report is three files in the task's record folder, named at the end of

`--compliance owasp,pci-dss` (at a shell) maps the findings to compliance frameworks — OWASP, PCI-DSS, SOC2, HIPAA, ISO27001 — and adds `sec-compliance.md`. A shell run's record folder is under `~/.codeaf/v3/carried/sec/`, one folder per run.

## A failed or incomplete audit — a model error, hunts that did not finish, no findings

If a hunt cannot finish because a model call failed or its session stopped, sec
records that failure separately from a completed hunt with no findings. Its ending,
account and readable report say **analysis incomplete** and how many hunts failed.
Findings from completed hunts still go through the same checks and remain in the
report; the failure itself is never a security finding. If all hunts fail, sec
makes no claim that the code has no exploitable problems.

The run ends as failed when any hunt fails, or on its ceiling if the run's dollar
or time limit caused the incomplete analysis. A run stopped or refused by the
model service later in the audit also says analysis incomplete, retaining any
findings already checked; a partial result never turns that stop into a pass.
The JSON report keeps the attempted
count and each failed strategy and reason in `metadata.hunt_coverage`; the SARIF
records unsuccessful execution and an error notification, alongside any findings.
Global stops are also recorded in `metadata.analysis_incomplete`. If the pipeline
cannot produce a result, the ending explains the stop without promising a report.

## An earlier audit's report — find it from another conversation, run it again, the last audit, start fresh

A sec run in the chat keeps its report in that task's record folder, `~/.codeaf/v3/projects/<project>/<conversation>/tasks/<n>/`. Every conversation in the project can see the run in `tasks`: its row's outcome is the first line of its account, which says what it found — `Security audit of the whole repository: 14 problems, 9 confirmed and 5 likely (1 critical, 1 high, 12 medium).` — and its `artifact` is that record folder, where `sec-report.md` is.
Expand Down
29 changes: 29 additions & 0 deletions internal/secaf/agents/hunt/coverage_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package hunt

import (
"context"
"errors"
"testing"

"github.com/Agent-Field/codeaf/internal/agentsession/appx"
"github.com/Agent-Field/codeaf/internal/secaf/config"
"github.com/Agent-Field/codeaf/internal/secaf/schemas"
)

func TestResumedHuntsKeepPartialFindingsAndFailures(t *testing.T) {
withStrategyTable(t,
strategyEntry{schemas.HuntStrategyInjection, func(context.Context, appx.Harnesser, string, schemas.ReconResult, config.DepthProfile) (schemas.HuntResult, error) {
return schemas.HuntResult{}, errors.New("cannot search")
}},
strategyEntry{schemas.HuntStrategyAuth, func(context.Context, appx.Harnesser, string, schemas.ReconResult, config.DepthProfile) (schemas.HuntResult, error) {
result := schemas.NewHuntResult()
result.Findings = []schemas.RawFinding{testFinding("app/a.py", "one")}
return result, nil
}},
)
withDedup(t, passthroughDedup)
result, err := RunHunt(t.Context(), &appx.Fake{}, ".", emptyRecon(), "standard", 2, 1, nil)
if err != nil || len(result.Findings) != 1 || result.Coverage == nil || result.Coverage.Attempted != 2 || len(result.Coverage.Failures) != 1 || result.Coverage.Failures[0].Strategy != "injection" {
t.Fatalf("partial hunt=%+v err=%v", result, err)
}
}
42 changes: 32 additions & 10 deletions internal/secaf/agents/hunt/hunt.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,10 +281,8 @@ func strategyValues(strategies []schemas.HuntStrategy) []string {
//
// Concurrency parity:
//
// - `asyncio.gather(..., return_exceptions=True)` means a hunter that raises
// is SILENTLY DROPPED — its findings are simply absent and nothing is
// logged. The Go port reproduces that: a per-index error slot that nothing
// reads. The only error RunHunt can return is dedup's.
// - A failed hunter contributes no findings, but codeaf records the failure
// separately so a resumed audit cannot mistake it for a completed search.
// - Results are flattened in strategy order regardless of completion order,
// so the port writes into a pre-sized slice rather than appending from the
// goroutines.
Expand Down Expand Up @@ -312,29 +310,32 @@ func RunHunt(
sem := semaphore.NewWeighted(int64(concurrencyLimit(maxConcurrentHunters, len(strategies))))

perStrategy := make([][]schemas.RawFinding, len(strategies))
failures := make([]error, len(strategies))
var wg sync.WaitGroup
for i, strategy := range strategies {
i, strategy := i, strategy
wg.Add(1)
go func() {
defer wg.Done()
if err := sem.Acquire(ctx, 1); err != nil {
// Python: a cancelled task is an exception gather swallows.
failures[i] = err
return
}
defer sem.Release(1)
findings, err := runSingleHunter(
ctx, strategyRunnerIndex[strategy], app, repoPath, recon, profile,
)
if err != nil {
// Python: `return_exceptions=True` then `if isinstance(result,
// Exception): continue`.
failures[i] = err
return
}
perStrategy[i] = findings
}()
}
wg.Wait()
if err := ctx.Err(); err != nil {
return schemas.HuntResult{}, err
}

allFindings := []schemas.RawFinding{}
for _, findings := range perStrategy {
Expand All @@ -352,6 +353,7 @@ func RunHunt(
deduplicated.ChainCount = len(deduplicated.Chains)
deduplicated.StrategiesRun = strategyValues(strategies)
deduplicated.HuntDurationSeconds = time.Since(started).Seconds()
deduplicated.Coverage = coverageForErrors(strategies, failures)
return deduplicated, nil
}

Expand Down Expand Up @@ -413,22 +415,23 @@ func RunHuntStreaming(
)

var wg sync.WaitGroup
for _, strategy := range strategies {
failures := make([]error, len(strategies))
for index, strategy := range strategies {
strategy := strategy
wg.Add(1)
go func() {
defer wg.Done()

if err := sem.Acquire(ctx, 1); err != nil {
failures[index] = err
return
}
hunterFindings, err := runSingleHunter(
ctx, strategyRunnerIndex[strategy], app, repoPath, recon, profile,
)
sem.Release(1) // Python releases before taking the dedup lock.
if err != nil {
// Python: gather(return_exceptions=True) swallows it, and the
// task never reaches its queue put.
failures[index] = err
return
}

Expand Down Expand Up @@ -461,6 +464,9 @@ func RunHuntStreaming(
}
wg.Wait()
close(findings) // Python: `await findings_queue.put(None)`.
if err := ctx.Err(); err != nil {
return schemas.HuntResult{}, err
}

unique := make([]schemas.RawFinding, 0, len(fingerprintOrder))
for _, fingerprint := range fingerprintOrder {
Expand All @@ -480,5 +486,21 @@ func RunHuntStreaming(
deduplicated.ChainCount = len(deduplicated.Chains)
deduplicated.StrategiesRun = strategyValues(strategies)
deduplicated.HuntDurationSeconds = time.Since(started).Seconds()
deduplicated.Coverage = coverageForErrors(strategies, failures)
return deduplicated, nil
}

// coverageForErrors retains selected invocations in order after all producers
// have joined. Complete runs keep the original report and checkpoint shape.
func coverageForErrors(strategies []schemas.HuntStrategy, failures []error) *schemas.HuntCoverage {
var failed []schemas.HuntFailure
for i, err := range failures {
if err != nil {
failed = append(failed, schemas.HuntFailure{Strategy: string(strategies[i]), Reason: err.Error()})
}
}
if len(failed) == 0 {
return nil
}
return &schemas.HuntCoverage{Attempted: len(strategies), Failures: failed}
}
3 changes: 3 additions & 0 deletions internal/secaf/agents/hunt/hunt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,9 @@ func TestRunHuntStreamingClosesOnHunterFailure(t *testing.T) {
if count != 0 {
t.Errorf("streamed %d batches, want 0", count)
}
if result.Coverage == nil || !result.Coverage.AllFailed() || result.Coverage.Attempted != 1 {
t.Fatalf("failed streaming hunt lost accounting: %+v", result.Coverage)
}
if result.TotalRaw != 0 || len(result.Findings) != 0 {
t.Errorf("result = %+v, want an empty one", result)
}
Expand Down
Loading
Loading