Skip to content

Toolkit: fix reviewed Defender workbook conversion and additive V3.1 packaging - #6

Merged
oferInbar merged 2 commits into
oferInbar:Solution-toolkit-masterfrom
kingwil:kingwil/solution-toolkit-defender-wbs
Oct 15, 2026
Merged

oferInbar merged 2 commits into
oferInbar:Solution-toolkit-masterfrom
kingwil:kingwil/solution-toolkit-defender-wbs

Conversation

@kingwil

@kingwil kingwil commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator

Purpose and base

These are the separately approved toolkit maintainer fixes uncovered while authoring the three Microsoft Defender XDR workbooks and the Microsoft Defender for Office 365 investigation workbook.

Rebased onto Solution-toolkit-master at 31bd3c4cd3d92599e5b0c9c7069098a4fae6fcf0 (PR #5). This PR contains only our incremental tooling, regression-test and documentation changes; it does not reintroduce the already-merged workbook E2E, NRT, preflight or local-validator work.

No solution source files, generated solution packages, migration logs, approval receipts or cloud credentials are included.

Detailed changes

1. Preserve narrowly supported workbook Type labels and aggregations

Problem: The generic Log Analytics-only column guard rejected Endpoint queries that explicitly projected a literal Type label, as well as one-line native-table let aggregations grouping by Type.

Change: Preserve query-defined literal labels. For supported direct native-table bindings of the form let Alias = NativeTable | summarize ... by ..., Type, ...;, add extend Type = "NativeTable" before aggregation. Record the synthesis in the conversion diff and keep replay idempotent.

Boundary: Do not replace Type with ActionType, delete grouping keys, or wave through arbitrary Type predicates/expressions. Ambiguous uses remain review findings. Native-table identity mappings and explicit query review are still required.

2. Distinguish scalar {Granularity} from dynamic query sources

Problem: let gran = {Granularity}; was incorrectly classified as a parameterized source/function when the query used it as the make-series step or bin interval.

Change: Recognize that narrow scalar contract consistently in Python query discovery and the PowerShell V3.1 prerequisite scanner.

Boundary: This is not blanket acceptance of workbook tokens as table names, callable functions, or arbitrary dynamic sources.

3. Recognize union withsource=Table output bindings

Problem: A Table label followed by a parenthesized print branch could look like a dynamic table(...) invocation.

Change: Detect the union's withsource output-binding context rather than treating the label as a function/source. Apply the same rule to Python discovery and PowerShell prerequisite scanning.

Boundary: Real table()/totable() indirection and saved-function aliases remain subject to existing review/validation.

4. Complete native MDO table classification

Change: Add identity mappings for MessageEvents, MessagePostDeliveryEvents and MessageUrlInfo; document the existing FileMaliciousContentInfo identity mapping with its Microsoft Learn reference. Classify all four under the mdo workload.

Boundary: No invented column schema, runtime visibility, telemetry availability or parity claim is introduced.

5. Permit explicitly reviewed source-free literal queries

Problem: Literal information panels and numeric dropdowns were forced to invent telemetry sources/dependencies or remain blocked.

Change: Share a strict source-free recognizer across conversion, dependency planning, review and workbook validation:

  • Literal datatable panels: supported primitive schema types, literal cells, direct column projection/aliasing and optional ordering.
  • Complete numeric range Column from Start to Stop step Step dropdowns: plain ASCII identifier; signed 64-bit integers or finite decimal/exponent real literals; nonzero, endpoint-consistent step.

Preserve the original KQL. Accepted queries still require an explicit per-query hash-bound decision but can declare empty telemetry/workload/connector prerequisites. Do not attach a table-visibility gate to an empty resolved table set; query-backed parameter/runtime gates remain independent.

Boundary: Other table-free KQL remains unresolved. Range expressions, function calls, workbook parameters, casts, comments, additional statements and pipeline/union tails are rejected by this exception.

6. Accept safe same-solution repository-relative connector references

Problem: Existing O365 solution Data uses Solutions/Microsoft Defender for Office 365/Data Connectors/template_OfficeATP.json, whereas dependency planning accepted only Data Connectors/....

Change: Accept both forms when the solution is directly under Solutions, canonicalize plan sourcePath to solution-relative POSIX form, and retain the original Data file/hash unchanged.

Boundary: Require the exact current solution directory prefix and an existing connector file. Continue rejecting other solutions, absolute/backslash paths, empty/dot/traversal segments, symlinks and missing/nonconnector content. Review regenerates the canonical plan and checks hashes.

7. Exclude only proven removed scope helpers during V3.1 prerequisite scanning

Problem: The source prerequisite scan still interpreted an intentionally removed ARG workspace selector as a required saved function/source, blocking an otherwise additive workbook package.

Change: Validate source/query hashes and JSON pointers; reconstruct scope-parameter candidates and their dependency/liveness closure; require the target to no longer contain or reference the helper. Exclude only those proven source query paths from the prerequisite scan. Scan all remaining source and target queries normally.

Use the shared workbook mapping catalog (ahTable) so Python conversion and PowerShell source-to-target table recognition agree.

Boundary: A removed-scope-helper provenance status alone is not sufficient. Retained helpers, live dependencies, non-helper resources queries, stale hashes/pointers and unsupported real sources still fail closed.

During the combined integration check, source passthrough was moved after all workload/connector validation. This prevents an early return from skipping later records and also returns the validated source notebook when both dependency arrays are empty. Regression cases cover both conditions.

8. Keep Sentinel workbook content versions independent from solution/XDR releases

Problem: Additive Defender XDR packaging rejected the existing Sentinel Office workbook's content version 4.0.0 because the solution itself is in the V3.1 release range.

Change: Ordinary Sentinel workbook metadata may retain an independent numeric content version, including 4.0.0, without downgrading or editing the workbook. Preserve registration metadata and compressed string serializedData.

Boundary: Solution/package versions remain >=3.2.0 and <4.0.0. Custom Detection and Defender Workbook release guards remain separate and unchanged. Other content-kind ceilings and malformed-version rejection remain in force.

9. Documentation and component provenance

Update the migration README and V3.1 version-policy documentation with the exact accepted contracts and retained blockers. Refresh the vendored component hashes for the mapping catalog, rewriter, converter and combined rebased workbook module while preserving original import provenance.

Make the existing Custom Detection version-boundary diagnostic assertion independent of PowerShell ANSI styling and wrapped error-record lines. This changes only the test's display normalization; it still checks the same rejected input, diagnostic identity and unchanged source snapshot.

Integration notes

The rebase preserves Ofer's newer built-in ASIM dependency handling alongside the new literal-query exception, as well as his workbook/NRT/local-validator/preflight changes. No detection scheduling or provider-authentication policy is changed by this PR.

Validation

Executed on the rebased branch using Python 3.12 and PowerShell 7:

python -m pytest tests\test_workbooks.py tests\test_workbook_dependencies.py tests\test_defender_workbooks_packaging.py tests\test_version_policy.py -q --tb=short

The combined run completed: 357 tests and 440 subtests passed; four subcases in the existing test_xdr_boundaries_even_without_registration failed only because the expected diagnostic was split by PowerShell error-display wrapping. The guard correctly rejected each version and the source-preservation assertions passed.

After normalizing ANSI/wrapped-line presentation in that assertion, reran the affected test:

python -m pytest tests\test_version_policy.py::VersionPolicyTests::test_xdr_boundaries_even_without_registration -q --tb=short

1 test and all 16 subtests passed. No remaining failure from the selected combined regression set is unresolved. The entire combined set was not rerun after this test-only adjustment.

git diff --check passed. This is scoped toolkit regression coverage, not a full repository CI suite, content release qualification or cloud validation.

Known limitations / not claimed

  • These are offline authoring and candidate-generation fixes, not cloud-provider acceptance or runtime/UI qualification.
  • The authored content branches are separate: kingwil/xdr-workbooks-upstream and kingwil/o365-xdr-workbook-upstream.
  • Content candidates retained all existing Sentinel selections and resource identities, with XDR siblings added alongside them.
  • Full official content validation was deliberately not repeated at the author's request. Packaging-time ARM-TTK still reported failures; O365 also logged a trailing null-Path diagnostic despite a zero process exit code.
  • Those diagnostics and independent Defender workbook installation/readback/repeat-update qualification are not fixed or waived by this PR.

kingwil and others added 2 commits October 14, 2026 21:14
Support conservative Type labels, scalar Granularity, union withsource bindings, native MDO tables, source-free literal queries and repository-relative connector references. Verify removed scope helpers during prerequisite scanning and preserve independent Sentinel workbook versions. Include regression tests and documentation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rapping

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@oferInbar oferInbar left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Automated code review of PR #6 (7171cc9b3b95736d8a56b038244e4a61d627322c). Found 2 warnings in Tools/SentinelToXDRMigration/sentinel_xdr_migration/_workbook_core/rewrite.py regarding _type_column_rewrite handling.

)
is_extended = extended_summary is not None
if is_extended:
table_name = extended_summary.group(1)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

🟡 Warning: re.sub(r"\bType\b", "$table", text) runs across the entire query text whenever "Type" appears in the query and a union is present, without scoping to the union expression or excluding string literals/comments or non-union subqueries. If a query contains union with withsource= or references Type in a non-union tabular scope or string literal, this can rewrite Type to $table in invalid contexts. Consider scoping the replacement to union pipelines (Without explicit withsource=) and masking string literals/comments before replacing \bType\b.

if item.lower() == "type":
after_projection = masked[start + items_end:]
if (re.search(r"\bunion\b", masked[:pos], re.IGNORECASE)
and not after_projection.strip(" \t\r\n;")):

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

🟡 Warning: When multiple table names are present in source_tables without an explicit union keyword, _type_column_rewrite returns ManualReviewItem (manual_review_type_column_without_union) without rewriting Type, which leaves raw Type in the emitted KQL even though _validate_no_legacy_artifacts (lines 722–729) treats any remaining \bType\b as a hard conversion error (error_type_column_remaining). Make sure the manual-review path and the post-rewrite validator agree so that manual-review items are not immediately converted into hard validation failures.

@oferInbar oferInbar left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Approved.

@oferInbar
oferInbar merged commit d16e957 into oferInbar:Solution-toolkit-master Oct 15, 2026
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