Skip to content

Conversation

@ChrisEdwards
Copy link
Collaborator

@ChrisEdwards ChrisEdwards commented Jan 9, 2026

Summary

This PR fixes status filtering for vulnerability search tools (search_vulnerabilities and search_app_vulnerabilities) by properly passing the status field to TeamServer's POST API. It also refactors session metadata filter parsing for type safety.

Key changes:

  • Add status field to ExtendedTraceFilterBody (the SDK's TraceFilterBody lacks it)
  • Make status values case-insensitive (reportedReported)
  • Simplify code by removing an unnecessary factory method
  • Parse sessionMetadataFilters directly to domain type (eliminates @SuppressWarnings("unchecked"))

Why

The vulnerability search tools accept a statuses parameter (e.g., Reported,Confirmed,Fixed), but this filter was never actually sent to TeamServer. The SDK's TraceFilterBody class doesn't have a status field, so even though we parsed and validated the parameter, we weren't including it in the POST request body.

Additionally, the sessionMetadataFilters parameter used Map<String, Object> requiring unsafe casts at runtime. This is a code smell that indicates missing domain modeling.

What Changed

1. Added status field to ExtendedTraceFilterBody

The SDK's TraceFilterBody is missing the status field that TeamServer's API supports. We extend it to add this field:

@Getter
@Setter
public class ExtendedTraceFilterBody extends TraceFilterBody {
  private Set<String> status;
}

2. Updated params classes to return ExtendedTraceFilterBody

Both VulnerabilityFilterParams.toTraceFilterBody() and SearchAppVulnerabilitiesParams.toTraceFilterBody() now:

  • Return ExtendedTraceFilterBody instead of TraceFilterBody
  • Set the status field from the parsed statuses

3. Made status filtering case-insensitive

Users can now specify statuses in any case:

  • reported → normalized to Reported
  • CONFIRMED → normalized to Confirmed
  • fixed → normalized to Fixed

This was implemented by enhancing StringListSpec.allowedValues() to automatically perform case-insensitive matching and normalization to canonical form.

4. Removed unnecessary withSessionFilters() factory method

The old ExtendedTraceFilterBody.withSessionFilters() method copied all fields from a source TraceFilterBody then added session parameters. This was over-engineered - we can simply:

  1. Create ExtendedTraceFilterBody directly in the params class
  2. Set session params directly on the returned object in the tool

This removes ~50 lines of code and makes the flow clearer.

5. Type-safe session metadata filters with UnresolvedMetadataFilter

The sessionMetadataFilters parameter accepts JSON like {"branch":"main","developer":["Ellen","Sam"]}. Previously this was parsed to Map<String, Object> requiring @SuppressWarnings("unchecked") casts:

// Before - unsafe cast at runtime
@SuppressWarnings("unchecked")
private List<String> normalizeFilterValue(Object value) {
  if (value instanceof String) {
    return List.of((String) value);
  } else if (value instanceof List) {
    return (List<String>) value;  // Unsafe!
  }
  return List.of();
}

Now we parse directly to an immutable domain record:

// After - type-safe at compile time
public record UnresolvedMetadataFilter(String fieldName, List<String> values) {
  public UnresolvedMetadataFilter {
    if (fieldName == null || fieldName.isBlank()) {
      throw new IllegalArgumentException("fieldName cannot be null or blank");
    }
    values = values == null ? List.of() : List.copyOf(values);
  }
}

The record name documents the data flow: these filters have user-provided field names that must be resolved to numeric IDs before API calls.

How It Works

Status filtering - Before:

User provides: statuses="Reported,Confirmed"
     ↓
VulnerabilityFilterParams parses and validates statuses
     ↓
toTraceFilterBody() creates TraceFilterBody WITHOUT status
     ↓
TeamServer receives: { severities: [...], environments: [...] }
                     // status missing!

Status filtering - After:

User provides: statuses="reported,CONFIRMED"
     ↓
VulnerabilityFilterParams parses, validates, and normalizes to ["Reported", "Confirmed"]
     ↓
toTraceFilterBody() creates ExtendedTraceFilterBody WITH status
     ↓
TeamServer receives: { severities: [...], environments: [...], status: ["Reported", "Confirmed"] }

Session metadata - Before:

JSON: {"branch":"main"}
     ↓
MetadataJsonFilterSpec.get() → Map<String, Object>
     ↓
normalizeFilterValue() with @SuppressWarnings("unchecked")
     ↓
TraceMetadataFilter(fieldId, values)

Session metadata - After:

JSON: {"branch":"main"}
     ↓
MetadataJsonFilterSpec.get() → List<UnresolvedMetadataFilter>
     ↓
Stream transformation (no unsafe casts)
     ↓
TraceMetadataFilter(fieldId, filter.values())

Step-by-Step Walkthrough

  1. ExtendedTraceFilterBody.java - Simplified from 65 lines to 19 lines

    • Removed withSessionFilters() factory method
    • Added simple status field with Lombok @Getter/@Setter
  2. VulnerabilityFilterParams.java - Returns ExtendedTraceFilterBody with status

    • toTraceFilterBody() now returns ExtendedTraceFilterBody
    • Sets status field using Set.copyOf(statuses) for immutability
  3. SearchAppVulnerabilitiesParams.java - Same changes plus type-safe filters

    • Returns ExtendedTraceFilterBody with status
    • Field type changed from Map<String, Object> to List<UnresolvedMetadataFilter>
  4. SearchAppVulnerabilitiesTool.java - Simplified session handling

    • No longer uses withSessionFilters() factory
    • Sets agentSessionId and metadataFilters directly on the filter body
    • Removed normalizeFilterValue() method and @SuppressWarnings
    • resolveSessionMetadataFilters() now takes List<UnresolvedMetadataFilter>
  5. StringListSpec.java - Case-insensitive validation with normalization

    • allowedValues() now automatically matches case-insensitively
    • Normalizes input to canonical form (e.g., reportedReported)
  6. MetadataJsonFilterSpec.java - Returns domain type

    • get() returns List<UnresolvedMetadataFilter> instead of Map<String, Object>
    • Parsing and validation happen in one place
  7. UnresolvedMetadataFilter.java - New domain record

    • Immutable with defensive copies
    • Validates field name at construction
    • Self-documenting: name indicates field name resolution is pending

Testing

  • ✅ 598 unit tests passing
  • ✅ New tests added for:
    • ExtendedTraceFilterBody.status field
    • toTraceFilterBody() returns correct type with status
    • Case-insensitive status normalization
    • UnresolvedMetadataFilter record validation
    • MetadataJsonFilterSpec returns List<UnresolvedMetadataFilter>

Files Changed

File Change
ExtendedTraceFilterBody.java Simplified to just status field
SearchAppVulnerabilitiesTool.java Direct session params, removed unsafe casts
SearchAppVulnerabilitiesParams.java Returns ExtendedTraceFilterBody, uses List<UnresolvedMetadataFilter>
VulnerabilityFilterParams.java Returns ExtendedTraceFilterBody with status
StringListSpec.java Case-insensitive allowedValues()
MetadataJsonFilterSpec.java Returns List<UnresolvedMetadataFilter>
UnresolvedMetadataFilter.java NEW - Domain record for type-safe filters
Tests Updated for new behavior

Statuses like 'reported', 'CONFIRMED', 'fixed' are now accepted and
normalized to the canonical form ('Reported', 'Confirmed', 'Fixed').

Added caseInsensitive() method to StringListSpec for reusable
case-insensitive validation with normalization.
When a StringListSpec has allowedValues set, case-insensitive matching
is now automatic - no need for explicit .caseInsensitive() call.
Values are normalized to canonical form.
- Add @Getter/@Setter to ExtendedTraceFilterBody, remove manual methods
- Use Set.copyOf() instead of new HashSet<>() for immutable status sets
- MetadataJsonFilterSpec.get() now returns List<UnresolvedMetadataFilter>
- SearchAppVulnerabilitiesParams uses List<UnresolvedMetadataFilter>
- Simplified resolveSessionMetadataFilters() to stream transformation
- Removed normalizeFilterValue() and @SuppressWarnings("unchecked")
- Updated all tests for new type signatures
@ChrisEdwards ChrisEdwards merged commit 678e685 into main Jan 11, 2026
4 checks passed
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.

3 participants