Skip to content

feat: Added Zone Display Title to Schema Findings Table - BED-8273#2826

Open
kpowderly wants to merge 12 commits into
mainfrom
BED-8273
Open

feat: Added Zone Display Title to Schema Findings Table - BED-8273#2826
kpowderly wants to merge 12 commits into
mainfrom
BED-8273

Conversation

@kpowderly
Copy link
Copy Markdown
Contributor

@kpowderly kpowderly commented May 26, 2026

Description

Problem Statement: The unified findings endpoint needed to return a human-readable title (e.g. "Tier Zero Write Owner") alongside the raw finding key (e.g. "T0WriteOwner") so the new Attack Paths table could display and sort/filter on it. Title resolution was split across two sources: a static title.md file first, then GetRemediationByFindingName().DisplayName as a fallback. Neither was tag-aware, meaning T0 and PZ customers could get the wrong label depending on context.

The ticket also surfaced two related questions: what is the intended difference between the short name (raw finding key) and the long name (display title), and why do title.md and the CUE-defined name differ post-OpenGraph.

What I found: The short name is the stable programmatic identifier; the long name is the human-readable label shown in the UI. They diverged when OpenGraph introduced custom (non-built-in) findings whose display names live only in the database. title.md and the CUE name are not the same because CUE names are schema identifiers, while title.md was added later as a UI-layer override. OpenGraph findings have no static file at all so their display name is entirely DB-driven.

The best course of action for sort/filter was to canonicalize the title into the schema_findings table as display_name and zone_display_name, making it a first-class database column rather than a runtime-resolved string.

Fix

See Jira Comment: https://specterops.atlassian.net/browse/BED-8273?focusedCommentId=44430

Motivation and Context

Resolves BED-8273

Because the unified findings endpoint was returning raw finding keys (e.g. T0WriteOwner) instead of human-readable titles in some cases, and when titles were resolved they weren't tag-aware. T0 and PZ customers could get the wrong label for the zone they were viewing.

How Has This Been Tested?

Integration + Unit Tests

Screenshots (optional):

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

Summary by CodeRabbit

  • New Features

    • Schema findings now include a zone display name: create, view, filter, sort, and update by zone.
  • Documentation

    • API docs and CSV examples updated to surface a human-readable, zone-aware title for attack-path findings.
  • Tests

    • Integration tests expanded to cover zone display name handling (create, update, filter, sort, null/empty cases).
  • Chores

    • Database migration added to persist zone display names.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 26, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 46ee1a25-d2e8-43c5-aaba-62f1f638c359

📥 Commits

Reviewing files that changed from the base of the PR and between ccb3ed5 and fe2e43a.

📒 Files selected for processing (8)
  • cmd/api/src/database/graphschema.go
  • cmd/api/src/database/graphschema_integration_test.go
  • cmd/api/src/database/migration/migrations/20260519000000_v9_add_schema_findings_title.sql
  • cmd/api/src/database/mocks/db.go
  • cmd/api/src/database/upsert_schema_finding.go
  • cmd/api/src/database/upsert_schema_finding_integration_test.go
  • cmd/api/src/model/graphschema.go
  • packages/go/openapi/src/paths/attack-paths.attack-paths-findings.yaml

📝 Walkthrough

Walkthrough

This PR adds support for a zone_display_name field to schema findings across database migration, CRUD APIs, data models, upsert wiring, integration tests, and OpenAPI docs to enable storing, querying, filtering, and sorting by zone-specific finding titles.

Changes

Schema Finding Zone Display Name Support

Layer / File(s) Summary
Database schema and CRUD methods
cmd/api/src/database/migration/migrations/20260519000000_v9_add_schema_findings_title.sql, cmd/api/src/database/graphschema.go, cmd/api/src/database/mocks/db.go
Migration adds nullable zone_display_name column. CreateSchemaFinding, GetSchemaFindings, and UpdateSchemaFinding extend method signatures and SQL to persist and retrieve the field. Mock database updated for test compatibility.
Data model and validation logic
cmd/api/src/model/graphschema.go
SchemaFinding struct gains ZoneDisplayName field using nullable string type. RelationshipFindingInput gains ZoneDisplayName string field. Sortable and filterable column validation extended to recognize zone_display_name with equality/inequality/approximate match operators.
Upsert application logic
cmd/api/src/database/upsert_schema_finding.go
Converts input ZoneDisplayName to nullable value and passes it through to CreateSchemaFinding during finding creation with remediation workflow.
Schema finding CRUD integration tests
cmd/api/src/database/graphschema_integration_test.go
Added test coverage for zone_display_name in creation, querying with filtering and sorting, and updates. Test cases verify NULL default behavior, custom non-empty values, and empty-string rollback to NULL semantics.
Upsert integration tests
cmd/api/src/database/upsert_schema_finding_integration_test.go
Extended finding creation and update tests to parameterize and assert ZoneDisplayName persistence through the upsert workflow.
OpenAPI specification
packages/go/openapi/src/paths/attack-paths.attack-paths-findings.yaml
Added title query parameter for filtering findings. Response schema includes title field describing zone-context variability. CSV export examples updated. Minor formatting adjustment to sortable columns description.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

enhancement

Suggested reviewers

  • cweidenkeller
  • benwaples
  • LawsonWillard
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: adding a zone display title field to the schema findings table, with specific reference to the Jira ticket (BED-8273).
Description check ✅ Passed The description is comprehensive, covering the problem statement, investigation findings, and the fix. All required sections are present with substantial detail, though the resolution link format differs from the template suggestion.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch BED-8273

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot added the api A pull request containing changes affecting the API code. label May 26, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cmd/api/src/database/graphschema.go`:
- Around line 952-960: The CreateSchemaFinding function currently binds
zoneDisplayName as a plain string so empty string "" gets inserted instead of
NULL; modify the function to convert zoneDisplayName into a nullable value
(e.g., sql.NullString{String: zoneDisplayName, Valid: zoneDisplayName != ""})
and pass that nullable variable into the Raw query instead of zoneDisplayName so
empty titles are persisted as NULL (update/create sql import if missing); keep
the rest of the INSERT/Scan logic targeting model.SchemaFinding unchanged.

In `@cmd/api/src/database/upsert_schema_finding_integration_test.go`:
- Around line 88-89: The test compares args.zoneDisplayName /
args.newZoneDisplayName (string) directly to finding.ZoneDisplayName
(null.String), which can fail; update the assertions to first assert
finding.ZoneDisplayName.Valid (or assert the expected Valid state when
empty-string should map to null) and compare the string via
finding.ZoneDisplayName.ValueOrZero(); replace direct comparisons in the
upsert_schema_finding_integration_test assertions with
assert.True/False(finding.ZoneDisplayName.Valid) as appropriate and
assert.Equal(t, args.zoneDisplayName, finding.ZoneDisplayName.ValueOrZero())
(and likewise for args.newZoneDisplayName).

In `@packages/go/openapi/src/paths/attack-paths.attack-paths-findings.yaml`:
- Around line 158-160: The OpenAPI examples for the response schema in
attack-paths.attack-paths-findings.yaml are missing the newly added title
property; update every JSON example object tied to the findings/response schema
to include a "title" string that matches the description (e.g., a human-readable
finding title such as "Tier Zero" or "Privilege Zone" variant). Locate the
examples under the findings response examples in
attack-paths.attack-paths-findings.yaml and add the title field to each example
payload so the documented examples match the schema's title property.
- Around line 133-135: The CSV example's data rows do not match the header
columns
(severity,finding,title,finding_type,platform,environment_id,zone_id,zone_name,source_principal_id,source_principal_kind,source_principal_name,target_principal_id,target_principal_kind,target_principal_name,status,first_seen,last_seen):
fix by realigning each data row to have 17 comma-separated values matching those
header fields (insert empty placeholders for missing values like
zone_id/zone_name or source/target principal fields, or reorder values to their
correct header positions) so the example rows at the two lines conform exactly
to the header schema.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 1c4c42f2-7e0b-4b61-87de-b2a4c2e30d35

📥 Commits

Reviewing files that changed from the base of the PR and between c0e56f5 and c32623f.

📒 Files selected for processing (8)
  • cmd/api/src/database/graphschema.go
  • cmd/api/src/database/graphschema_integration_test.go
  • cmd/api/src/database/migration/migrations/20260519000000_v9_add_schema_findings_title.sql
  • cmd/api/src/database/mocks/db.go
  • cmd/api/src/database/upsert_schema_finding.go
  • cmd/api/src/database/upsert_schema_finding_integration_test.go
  • cmd/api/src/model/graphschema.go
  • packages/go/openapi/src/paths/attack-paths.attack-paths-findings.yaml

Comment thread cmd/api/src/database/graphschema.go
Comment thread cmd/api/src/database/upsert_schema_finding_integration_test.go Outdated
Comment thread packages/go/openapi/src/paths/attack-paths.attack-paths-findings.yaml Outdated
@kpowderly kpowderly self-assigned this May 28, 2026
@coderabbitai coderabbitai Bot added the enhancement New feature or request label May 28, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

Actionable comments posted: 0

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

Actionable comments posted: 0

@kpowderly
Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

✅ Actions performed

Full review triggered.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

Actionable comments posted: 0

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

Labels

api A pull request containing changes affecting the API code. enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant