Skip to content

FEAT: Findings dashboard with persistent Operations in the GUI - #3066

Open
Adrian Gavrila (adrian-gavrila) wants to merge 3 commits into
microsoft:mainfrom
adrian-gavrila:adrian-gavrila-findings-v1
Open

Adrian Gavrila (adrian-gavrila) wants to merge 3 commits into
microsoft:mainfrom
adrian-gavrila:adrian-gavrila-findings-v1

Conversation

@adrian-gavrila

@adrian-gavrila Adrian Gavrila (adrian-gavrila) commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Description

This adds a findings dashboard to the GUI. A red teamer can now write a finding while testing (title, severity, an optional description and harm type), attach the conversations that prove it, and see everything an Operation turned up on one page.

operation-detail

To make that work, Operations become real records. They used to be a free-text label on attacks and scanner runs, so "Contoso Q3" and "contoso q3 " counted as two different engagements and there was nothing to attach a finding to. Now there's an OperationEntries table with a stable id and unique names (case and surrounding whitespace ignored). Findings, evidence links and per-Operation counts hang off that id. It also sets up a later change that gives attacks and scanner runs a proper operation_id foreign key in place of the label match.

operations-list

What's in it:

  • Memory: OperationEntry, FindingEntry and FindingEvidenceEntry tables, plus one Alembic migration (c8d3e5f7a901). Its downgrade refuses to run once any Operation exists, so it can't silently drop data. Harm type uses the existing HarmCategory enum. Findings are ordered by severity, then newest first, and paginated.
  • Backend: REST routes for Operations, findings (create, edit, delete) and conversation evidence links. The Operations list includes finding counts by severity.
  • Toolbar: an Operation picker with a New operation… entry. It only accepts saved Operations. The selected Operation labels new attacks and scanner runs, the same way the label did before.
  • Operations pages: a list styled like Attack history, and a detail page with findings, expandable evidence and a link to execution history filtered to that Operation.
  • Chat: a conversation can be attached to a finding from the chat view.
  • Docs: doc/gui/0_gui.md covers Operations and findings.

Severities are critical, important, moderate, low, informational, and "other" with required free text.

Not in this PR:

  • operation_id foreign keys on attacks and scanner runs. The link is still the Operation name label.
  • Moving an existing conversation to a different Operation.
  • Conversation and scanner-run counts on the Operations list (they need the foreign key above).
  • Configurable finding fields. That's a follow-up PR stacked on this one.

Tests and Documentation

  • New unit tests cover the models, the migration (upgrade, plus the guarded downgrade), the memory queries, the backend routes and services, and the frontend components.
  • pytest tests/unit/memory tests/unit/backend: 3530 passed, 5 skipped.
  • npm test -- --runInBand (frontend): 112 suites, 2847 passed
  • pre-commit run --files <changed files>: passed.
  • Tried by hand against an isolated SQLite trial database: creating Operations, filing findings from chat, linking and removing evidence, editing and deleting findings, and filtering history by Operation.
  • Not run: the new Playwright e2e specs (frontend/e2e/operations.spec.ts, labels-operation-picker.spec.ts) and anything against SQL Server.
  • Docs: doc/gui/0_gui.md updated.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
let ignore = false
operationsApi.list()
.then(result => {
if (!ignore && !result.items.some(operation => operation.name === currentValue)) onMissingRef.current()

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.

🔴 Must Fix: Please don't silently remove an existing Operation when it isn't in the new table.

On upgrade, an existing preference or server default such as operation: "contoso_q3" has no OperationEntries row because the migration doesn't import existing labels. This check calls onMissing, which removes the label and persists that removal in the browser. Subsequent attacks and scans then lose their Operation attribution without the user choosing to clear it.

I reproduced this in the browser. The updated keeps a legacy operation removable without offering it as a saved choice test also fails at line 314: it expects op_chosen_elsewhere but receives an empty value.

Please preserve the existing attribution until the user explicitly changes it, or provide an explicit import/register flow before replacing the label picker. Add an upgrade test covering both an existing browser preference and a configured default when the new table is empty.

...DEFAULT_HISTORY_FILTERS, operation: [current.operation.name],
})}`}>View execution history</Link>
<Text>Human assessments, separate from attack outcomes and scores.</Text>
{notice && <MessageBar intent="success" aria-live="polite"><MessageBarBody>{notice}</MessageBarBody></MessageBar>}

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.

🟡 Should Fix: The success message can overlap the finding it is supposed to confirm.

I reproduced this at 390px width by saving a finding with a roughly 195-character title and a long description. The MessageBar shrinks to 36px high while its text needs about 120px, so the confirmation is drawn over the next finding's heading and severity. It also spills outside the background on desktop.

This section is a height-constrained flex column, so the status bar needs to keep its natural height. Please prevent the MessageBar from shrinking, or use a short confirmation such as Finding saved with the finding title shown separately. Add a browser assertion using long content, since jsdom won't catch the overlap.

listbox={{ className: styles.harmListbox }}>
<Option value="">Not set</Option>
{harmTypes?.filter(category => category.toLowerCase().includes(harmSearch.toLowerCase()))
.map(category => <Option key={category} value={category}>{category}</Option>)}

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.

🟡 Should Fix: Keep the custom-category option available when the search has no match.

For example, typing organization-specific-risk filters out every category, including Other. The dropdown then says Choose Other to enter custom text, but the only enabled choice is Not set. I confirmed this in the GUI.

Please keep Other outside the filtered list, or add a selectable Use this as a custom category action that selects Other and fills the custom-text field. Add a test that searches for an unknown category and completes that flow without clearing the search.

<DialogActions>
<Button className={styles.button} disabled={submitting} onClick={() => { setOpen(false) }}>Cancel</Button>
<Button className={styles.button} appearance="primary"
disabled={submitting || !selection || settledKey !== requestKey || Boolean(listError) || disabled}

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.

🔴 Must Fix: Keep keyboard focus inside the attachment dialog while submitting and after a failure.

I reproduced this by selecting a finding and making the evidence POST return 503. Disabling the focused Attach button moves focus to document.body. When the request fails, the error is visible but focus stays outside the modal, and pressing Escape no longer closes it.

Please use disabledFocusable for the submitting action, as the finding form already does, or explicitly restore focus inside the dialog on failure. The submission guard already prevents duplicate requests. Also announce the attachment error with a live region. Add a browser test that fails an attachment, checks that focus remains inside the dialog, and dismisses it with Escape.

}}>
{page?.items.map(item => <Radio key={item.id} value={item.id} disabled={submitting} label={
<span title={`Finding ID: ${item.id}`}>
<span className={styles.findingHeading}>{item.title}

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.

🟡 Should Fix: Constrain and wrap long finding titles and custom classifications.

The form accepts these values, but the attachment picker does not keep them within the label width. With a long title/custom severity, I measured a radio row about 1,498px wide inside a 326px mobile dialog. The user has to scroll horizontally to read the choice. The detail page has a related problem at OperationDetailPage.tsx:154: custom severity text needs about 80px of height inside a 20px Badge and overlaps the title and timestamp.

Please give the radio/label flex children a shrinkable width and wrap long text. Custom severity text also needs a wrapping, auto-height presentation instead of a fixed-height Badge, on both the detail page and picker. Add desktop and narrow-screen browser coverage with long title and Other-classification values.

This branch has not been deployed

No deployments
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.

2 participants