Skip to content

feat(reports): add the detection-coverage pillar so the generated CISO report matches the rollup skill - #102

Merged
ubercylon8 merged 2 commits into
mainfrom
feat/report-detection-pillar
Aug 12, 2026
Merged

feat(reports): add the detection-coverage pillar so the generated CISO report matches the rollup skill#102
ubercylon8 merged 2 commits into
mainfrom
feat/report-detection-pillar

Conversation

@ubercylon8

@ubercylon8 ubercylon8 commented Aug 12, 2026

Copy link
Copy Markdown
Owner

What & why

Wires sentinel-mcp into the automated report generator, closing a divergence
opened when server #9 landed: skills/cross-platform/ciso-risk-rollup/SKILL.md
gathers a detection-coverage pillar, but scripts/report_gather.py had only
six _pillar_* factories — so the agent-driven rollup covered seven pillars while
the generated CISO report covered six. docs/user-guide/workflows.md documented
that gap rather than hiding it; this PR removes the gap and the caveat.

CISO — seventh pillar

detection_coverage, from Sentinel's get_detection_coverage. It answers a
question no other pillar does: how much of ATT&CK do our own analytics rules
actually cover?
— and it separates operator-authored rules (Scheduled, NRT)
from Microsoft-managed ones (Fusion, MicrosoftSecurityIncidentCreation,
MLBehaviorAnalytics, ThreatIntelligence), because the overall number flatters.
On the validation workspace the honest reading was "5 rules (5 enabled, 4 custom)
— 2 of 14 MITRE tactics covered by custom rules (12 covered overall, incl.
Microsoft-managed rules)"
.

detection_engineer — analytics-rule inventory

That persona gathered alerts, incidents, LimaCharlie D&R rules, endpoint detections
and PA weak techniques, but had no Sentinel analytics-rule coverage at all
a real gap for the persona whose entire job this is. Added as analytics_rules.

The design decision

get_detection_coverage returns a summary finding plus up to 25 per-rule
findings
. Every other CISO pillar contributes exactly one headline number, and 25
rule entries would swamp an executive rollup.

So the two personas consume the same tool differently:

  • CISO → summary only (findings[:1]), one headline like every sibling pillar.
  • detection_engineer → the full result, rule inventory included, which is where
    that detail belongs.

This split is mutation-tested: making the CISO factory return the full list fails
test_ciso_pillar_keeps_only_the_summary_finding.

Degradation

No wrapper needed. get_detection_coverage already returns
"Sentinel detection coverage unavailable — ARM coordinates not configured" when
the ARM triple is unset, and sections.is_not_assessed matches on
DEGRADATION_MARKERS, which contains "not configured" — so a logs-only
deployment renders the pillar as NOT ASSESSED and the rollup carries on, which
is exactly the "handle a dark pillar gracefully" behaviour the skill specifies.
Verified rather than assumed.

i18n

New group keys need labels in both blocks, and the EN block carries an explicit
constraint that existing English values must stay byte-identical so the frozen
goldens do not move:

key EN ES
group_detection_coverage Detection coverage Cobertura de detección
group_analytics_rules Analytics rules Reglas analíticas

No golden file changed.

Docs

docs/user-guide/workflows.md, skills/reports/generate-report/SKILL.md,
.opencode/agents/ciso.md, and report_gather.py's own module docstring and
GATHER_MAP comment all said "six". Updated. The remaining "six-pillar" references
live in docs/superpowers/ specs and plans, which are historical records of the
design at the time and are deliberately untouched.

Note ciso-risk-rollup/SKILL.md's "a partial rollup across the remaining six
pillars is still valuable" is correct as written — with seven pillars, one dark
leaves six.

Checklist (mirrors the Critical Rules in CLAUDE.md)

  • Read-only by default — the new pillar calls one read tool.
  • Returns the findings schema — unchanged.
  • Redaction at the boundary — unchanged; the report path already used redact_finding.
  • Safety logic stays in core/scripts/report_gather.py is the designated
    platform-aware layer (core/reports stays platform-free); no safety logic added.
  • Small-model-safe — no tool or schema change.
  • Eval task added — no new tools.
  • No secrets staged — verified.
  • uv run pytest (1010 passed, 1 skipped), ruff, mypy clean; gen_docs.py no diff.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XYTy7da8Z5ZHhkwcCpjojZ

ubercylon8 and others added 2 commits August 12, 2026 07:31
…on-engineer report pillars

The CISO risk-rollup skill gathers a headline detection-coverage number from
sentinel-mcp, but scripts/report_gather.py only had six _pillar_ factories, so
the generated CISO report silently omitted it. Adds _pillar_detection_coverage
(CISO: summary finding only, matching every other one-headline pillar) and
_sentinel_analytics_rules (detection-engineer: full result incl. per-rule
findings, where the rule inventory belongs). Adds EN/ES i18n labels for the
two new group keys.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XYTy7da8Z5ZHhkwcCpjojZ
docs/user-guide/workflows.md, skills/reports/generate-report/SKILL.md, and
the opencode CISO persona prompt all documented the six-pillar / "Sentinel
not yet wired" divergence between the agent-driven rollup skill and the
generated report. Now that report_gather.py has the seventh pillar, update
all three to say seven and name Sentinel explicitly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XYTy7da8Z5ZHhkwcCpjojZ
@claude

claude Bot commented Aug 12, 2026

Copy link
Copy Markdown

Claude finished @ubercylon8's task in 1m 20s —— View job


Security review — PR #102

  • Gather context (PR diff, changed files)
  • Review scripts/report_gather.py (new pillar logic)
  • Review core/f0_sectools_core/reports/i18n.py (i18n additions)
  • Review scripts/tests/test_gen_report.py (test coverage)
  • Review docs/skill changes for accuracy
  • Post final findings

Summary: no security-significant issues found. This is a low-risk wiring PR — it adds a 7th CISO pillar and a detection-engineer group by calling an existing, already-reviewed sentinel-mcp tool (get_detection_coverage); no new tool, schema, or write path is introduced.

Checked against the CLAUDE.md Critical Rules:

  • Read-only by default_pillar_detection_coverage / _sentinel_analytics_rules (scripts/report_gather.py:116-183) only call the existing read tool get_detection_coverage. No gated-write surface touched.
  • Secrets isolation — follows the exact per-platform pattern used by every other factory: load_dotenv(".env.sentinel")SentinelConfig.from_env("SENTINEL")async with SentinelClient(cfg) as c. Consistent with the other 12 factories in the file; no cross-platform bleed.
  • Redaction at the boundary — both new factories return through the shared _run_group wrapper (scripts/report_gather.py:342-351), which applies redact_finding to every group's output uniformly, and the exception path (_degraded, line 346) already runs redact_text(...)[:300] on the stringified exception. The new pillars get this for free — no bespoke (and therefore no bypassable) redaction logic was added.
  • Findings schema — unchanged; both factories just filter/pass through list[Finding].
  • Small-model-safe — no new tool or argument schema; window_hours stays the only (existing) scalar param.
  • CISO summary-trim correctnessfindings[:1] (report_gather.py:129) is a slice, not an index, so an empty get_detection_coverage() result degrades to [] rather than raising — verified by test_ciso_pillar_defensive_on_empty_result. The CISO/detection-engineer split (summary-only vs. full inventory) is directly covered by test_ciso_pillar_keeps_only_the_summary_finding and test_detection_engineer_analytics_rules_keeps_the_full_result.
  • Degradation path verified, not assumedtest_sentinel_detection_coverage_unconfigured_is_not_assessed exercises the real is_not_assessed/DEGRADATION_MARKERS matching end-to-end with a stub client (no env vars, no network), confirming a dark Sentinel pillar renders as NOT ASSESSED rather than crashing the rollup — good practice, avoids an unverified assumption.
  • i18n — new group_detection_coverage / group_analytics_rules keys added to both EN/ES blocks; existing EN values untouched, so frozen goldens don't move.

Minor, non-blocking nit: the PR description references a test named test_ciso_detection_pillar_is_summary_only, but the actual test added is test_ciso_pillar_keeps_only_the_summary_finding (scripts/tests/test_gen_report.py:488). Same coverage, just a naming drift between the PR body and the code — worth a quick fix to the description for anyone searching by that name later, not a code issue.

No inline comments — nothing rose to the level of a specific line-level finding.

@ubercylon8
ubercylon8 merged commit c381897 into main Aug 12, 2026
11 checks passed
@ubercylon8
ubercylon8 deleted the feat/report-detection-pillar branch August 12, 2026 11:39
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.

1 participant