feat(skills): add TM7 generation and native validation harness to security-planning - #2574
Open
Bill Berry (WilliamBerryiii) wants to merge 20 commits into
Open
feat(skills): add TM7 generation and native validation harness to security-planning#2574Bill Berry (WilliamBerryiii) wants to merge 20 commits into
Bill Berry (WilliamBerryiii) wants to merge 20 commits into
Conversation
…urity-planning - generate deterministic .tm7 models from a declarative threat-model spec - add containment-preserving layout with viewport-aware surface sizing - add native TMT validation harness with per-surface metrics and screenshots - add layout overlay schema for fingerprint-guarded manual adjustments - commit hve-core threat-model spec under docs/planning/threat-models Closes #2567 🔒 - Generated by Copilot
- Add tracked comprehensive-spec.yaml fixture; drop .copilot-tracking paths - Render bases from the spec instead of an untracked 1.4 MB model - Share one rendered base per module to cut redundant generation
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF ScorecardScorecard details
Scanned Files
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2574 +/- ##
==========================================
- Coverage 83.10% 82.74% -0.37%
==========================================
Files 164 152 -12
Lines 22254 22188 -66
Branches 29 0 -29
==========================================
- Hits 18495 18359 -136
- Misses 3756 3829 +73
+ Partials 3 0 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
Eval Execution |
- Lock pytest-mock 3.15.1 from PyPI for mocker-based test coverage
- Match TM7 members by exact local name; Id and TypeId no longer collide - Key authored-base index on SemanticId so relabelled flows resolve - Remove the two unreferenced serializers
…alization - Own slug derivation and mitigation text in tm7_threat_contract - Attach resolved mitigations to generated threat instances - Normalize Path and datetime in both fingerprint copies
…e loss - Raise when a spec threat target_ref resolves to nothing - Reject suppressing a node that still carries connectors - Verify the KnowledgeBase placeholder before substitution
Layout could place a node outside its own trust boundary while still reporting success. The only bound was a whole-surface check against twice the viewport, which rejects gross overflow but says nothing about whether a node stayed inside the zone that owns it. A moderately dense zone therefore emitted a diagram whose trust boundaries no longer described the model. Add `_assert_zone_containment` to fail closed when a node escapes its zone, and correct the sizing defects the guard exposed: - reserve the label band, padding, and node band that `_allocate` actually consumes instead of a flat 96 points - honor measured subtree requirements when sizing the root canvas, which previously used only node-count and depth heuristics - give each sibling zone its own requirement in `_pack_zone_rects` and share only the surplus, rather than splitting the parent evenly - stop `max(120.0, ...)` floors from letting a content box extend past the zone that owns it, so an insufficient allocation reaches the existing fail-closed check - clamp node width to its lane and keep the contextual lane inside the zone The shipped `threat-model-spec-example.yaml` was rendering `ds-01` 22.8 points outside `tz-01`. Comprehensive-spec output is byte-identical, so the sizing corrections are confined to zones that were previously mis-sized. Refs finding 25
`write_tm7` used `Path.write_text`, which truncates the destination before the new content lands. A failing write therefore left the previous model at zero bytes rather than intact. Both writers now stage a sibling temporary file, flush and fsync it, then move it into place with `os.replace`, and unlink the temporary file if anything raises. The replace also swaps a symlinked destination instead of writing through it to the link target. `generate_tb7` additionally: - derives threat metadatum identifiers from the property name instead of `uuid.uuid4()`, which contradicted the module's deterministic contract and made two runs over identical input produce non-comparable templates - creates the output parent directory, so a nested destination no longer raises an unhandled FileNotFoundError that escapes the CLI as a traceback The durability test injects failure with an unencodable lone surrogate rather than by patching the write path, so it describes the contract rather than the implementation. Refs findings 21, 23, 65
`populate_tm7_threats` asserted a hardcoded 80 unique threat ids with no override, so every internally consistent model that declared a different number was rejected. The count is now an optional `expected_threat_count` argument with a matching `--expected-threat-count` flag, defaulting to accepting any consistent spec and failing only when the caller's own number disagrees. Also: - catch `ThreatContractError` at the `generate_tm7` CLI boundary so an invalid threat state reports a concise message and a nonzero exit code instead of escaping as a traceback - resolve markdown template profiles from the package root, matching `generate_tm7`; deriving the directory from the spec location let an external spec resolve a different profile than generation used - emit completeness warnings through a module logger so importing callers can capture or silence them, with logging configured at the CLI boundary to preserve the operator-visible stderr output `test_given_incomplete_spec_when_emit_then_ctm_warnings` now asserts log records rather than captured stderr, because pytest's logging plugin intercepts them. Operator-visible stderr is covered separately by a CLI test that runs the real subprocess. Refs findings 4, 19, 24, 45
`_validate_feedback_candidate` returned a hardcoded `"semantic_regression": False`, and its only caller recomputed the value solely when it was None. The gate was therefore dead code: `_evaluate_semantic_regression` never ran in the production feedback path, so a candidate that changed the model's semantic identity could still reach `automated-ready-pending-human`. The producer now returns None. It validates one candidate in isolation and holds no baseline, so it cannot decide whether identity regressed; deferring to the caller that owns the baseline makes the existing guard live rather than adding a second evaluation path. The new test patches the inner `_validate_candidate` seam rather than `_validate_feedback_candidate`, so the real feedback-candidate body and the real evaluator both execute. Reverting only the production change fails the test at the status assertion, confirming it characterizes the defect rather than the implementation. Refs finding 2
`evaluate_convergence` returned the success stop reason whenever `gate_failure_count == 0` and only checked `evidence_complete` afterwards, so a run that captured nothing reported `automated-ready-pending-human` on the strength of having observed no failures. The evidence check now runs first. Also: - catch `GenerationError` around candidate regeneration in the feedback loop, which previously escaped `run_harness` entirely and left no status.json - drop the six constant-0.0 members from `score_surface_layout_candidate`. Each needs laid-out geometry that a candidate never carries, so they could not be computed and, being constant, could never order two candidates; the stop decision ran on a score blind to six of the eleven dimensions it named - return `matches[0]` from `select_surface_tab`. The previous expression indexed `tabs` by `matches.index(matches[0])`, which is always 0, so on the raw-control path every surface resolved to the first tab and all captured evidence was attributed to it - gate the overlay write on a successful outcome. A ready status without an overlay is downgraded rather than reported as success, and a stopped run no longer publishes an overlay accumulated from an earlier clean iteration Refs findings 3, 20, 27, 29, 32
Executable discovery built roots from unvalidated environment variables, applied no signature check, and selected `max(..., key=st_mtime)`. Newest modification time is not a trust signal, so a decoy dropped into an allowed root wins on timestamp alone. Roots must now be absolute, acceptance requires a valid Authenticode signature naming CN=Microsoft Corporation, and selection is deterministic over trusted candidates preferring the pinned version. Redaction was inert on its real sink shapes. Measured against the previous implementation, five of eight representative credential shapes survived. The worst case was `Authorization: Bearer <jwt>`: the pattern's `\S+` consumed only the word "Bearer" and published the JWT, so the output looked redacted while leaking the secret. The key and value patterns now cover secret, credential, account key, connection string, private key, and signature shapes, the value run stops at a separator, a bare bearer or basic credential is matched without a preceding key name, and any query carrying a sensitive parameter is dropped whole. Also: - redact `write_csv_export` rows, the only sink that persisted verbatim - require a `.tm7-harness-owned` marker before recursive deletion and report cleanup errors instead of suppressing them with `ignore_errors=True` - require a complete replay invalidation block. The fingerprints were synthesized with `setdefault` from the same context they are validated against, so an overlay that omitted the block validated unconditionally; the evasion was deletion rather than forgery `_authenticode_subject` tries PowerShell 7 before Windows PowerShell because `Microsoft.PowerShell.Security` fails to load under the latter in some environments. A signature that cannot be established is treated as untrusted, never as a pass. Refs findings 7, 8, 9, 38, 39
…loading Each XML reader carried its own DTD and entity guard, and every copy scanned raw bytes for `<!DOCTYPE`. That only matches UTF-8; the same document encoded as UTF-16 interleaves NUL bytes, so the marker never appeared and the guard silently passed. `generate_tb7` had no policy at all and rejected an undefined entity only as an incidental ElementTree behavior. All four readers now call `tm7_threat_contract.parse_hardened_xml_bytes`, which decodes through the encodings XML permits before scanning, prefers defusedxml, and converts parser and defusedxml failures alike into `UnsafeXmlError` so no unsafe document escapes as a traceback. `Deserialize-Tm7.ps1`: - select the assembly directory by verifying it resolves the required type rather than by sort order. One ClickOnce payload directory carries only the local-storage assembly, and both share a timestamp, so the previous newest-write-time selection chose between them arbitrarily - confine assembly resolution to the selected directory and reject names carrying path separators or traversal - require absolute roots so an unset environment variable cannot contribute a directory beside the working directory - narrow the relaunch from `-ExecutionPolicy Bypass` to `RemoteSigned`, which still enforces signature checks on files carrying mark-of-the-web Also resolves two CodeQL "empty except" alerts in the fuzz harness by explaining why a rejected payload is the expected outcome. Refs findings 35, 36, 40
Dependency Review failed the security-planning skill's Windows-only group. All four packages ship permissive licenses that already appear in allow-licenses; the action cannot match them because the dependency graph reports a compound expression or "unknown". Verified against the installed wheels: - pywinauto declares BSD-3-Clause AND LGPL-2.1-only AND LGPL-2.1-or-later. The distributed wheel is BSD-3-Clause only: METADATA declares "License: BSD 3-clause" with the OSI BSD classifier, the bundled LICENSE is the BSD 3-clause text, and the package contains no LGPL reference. The LGPL components are detected in the upstream source repository and are not redistributed. This matches the existing shapely entry, whose LGPL component likewise refers to material outside the distributed code. - comtypes is MIT, pillow is MIT-CMU, pywin32 is PSF; each is already an allowed license type but reports "unknown" through the graph. The harness is Windows-only, opt-in, and not distributed as a product; portable TM7 generation requires none of these packages.
The fixtures README named an upstream repository and file but never said which local fixture came from it, and claimed the copy was "stored unmodified". The notices described the Microsoft threat-modeling templates as "reference-only". Both claims were wrong. Provenance was established by comparing every local .tm7 fixture against every .tm7 in the upstream repository at a recorded revision. Exactly one matched: tmt-reference.tm7 is gholliday/tm7-cli samples/demo.tm7 at 715954acc5b0a42386d3c0a3a42cdf35c5f41cfc, MIT licensed. It is not byte-identical: three LF line breaks inside <b:string> elements are stored as CRLF, which accounts for the entire 3-byte size delta. With whitespace normalized the two files are byte-identical at 1190991 bytes, so the difference is a checkout line-ending translation rather than an edit. The other three fixtures match nothing upstream and are first-party. default.tb7 and default-kb.xml are tracked and redistributed, and the knowledge base is embedded into every generated model, so "reference-only" understated the obligation. Both files are now named with SHA-256 digests. assets/templates/LICENSE was missing the final period of the MIT text. No fixture or template bytes changed; all six SHA-256 digests are identical before and after. Refs findings 15, 16, 48, 69, 72
The gap register used topic-prefixed IDs (G-EXEC, G-UIA, G-EVD, G-OVER,
G-PATH, G-TRUST) and bare severity words, both of which the skill security
model rules prohibit: IDs must be G-{TOKEN}-{N} with STRIDE-aligned tokens,
and the Severity column must carry a bare {Category}-{Level} token. IDs are
remapped to G-SPF-1, G-TAM-1, G-INF-1, G-TAM-2, G-TAM-3, G-EOP-1, G-REP-1,
and G-DOS-1, with severities restated as Spoofing-High, Tampering-Med,
InfoDisc-Med, EoP-Med, Repudiation-High, and DoS-Med. Every cross-reference
in the risk-rating tables is updated to match.
The summary claimed 6 open residual gaps while the register listed 8.
The model claimed "pane-scoped capture" in six places. The implementation
calls ImageGrab.grab(window=handle) against the Threat Modeling Tool window,
so capture is window-scoped: everything the tool displays is captured, not
just the diagram pane. The claims now state window-scoped behavior, and the
information-disclosure row is downgraded from "Mitigated" to "Partially
mitigated" because text redaction does not apply to image content.
Mitigation prose for executable trust and overlay replay is restated against
the controls implemented in P03: absolute installation roots with an
Authenticode publisher check and deterministic selection, and a required
complete invalidation fingerprint block.
Refs findings 12, 37, 41, 57
Katrien De Graeve (katriendg)
requested changes
Aug 3, 2026
Katrien De Graeve (katriendg)
left a comment
Katrien De Graeve (katriendg)
left a comment
Contributor
There was a problem hiding this comment.
Thanks for the thorough and well-documented work here — the portable-vs-Windows-only split, the human-confirmation gates, and the attribution effort all show real care. This review found two High-severity issues worth resolving before merge: validate_tm7_with_tmt.py has two XML entry points (_parse_xml and read_expected_surfaces) that were not migrated to the shared tm7_threat_contract.parse_hardened_xml_bytes hardening introduced in commit e6fe27d7, reproducing the same XXE-class bypass that commit set out to close everywhere. Separately, independent SHA-256 recomputation shows the digests recorded in THIRD-PARTY-NOTICES for default.tb7 and default-kb.xml don't match the committed bytes — worth a fix given this PR's stated goal of exact attribution.
A few Medium items round this out: an unanchored substring match in the Authenticode publisher check, three failing CI checks with no explanation in the PR description, and some duplicated helper functions worth consolidating into the existing shared tm7_threat_contract module. Full findings and suggested fixes are in the linked review. Some smaller ones, close when irrelevant.
Comment on lines
+577
to
+590
| def _parse_xml(path: Path) -> ET.Element: | ||
| data = path.read_bytes() | ||
| if b"<!DOCTYPE" in data.upper() or b"<!ENTITY" in data.upper(): | ||
| raise HarnessFailure( | ||
| "TM7 input contains DTD or entity declarations", | ||
| EXIT_ERROR, | ||
| ) | ||
| try: | ||
| if DefusedET is not None: | ||
| return DefusedET.fromstring(data) | ||
| return ET.fromstring(data) | ||
| except ET.ParseError as exc: | ||
| raise HarnessFailure(f"Unable to parse TM7 input: {exc}", EXIT_ERROR) from exc | ||
|
|
Contributor
There was a problem hiding this comment.
[Security/Standards] High — unmigrated XML hardening (CWE-611)
_parse_xml guards DOCTYPE/ENTITY with a raw byte scan (b"<!DOCTYPE" in data.upper()), which only matches UTF-8-family encodings. Commit e6fe27d7 introduced tm7_threat_contract.parse_hardened_xml_bytes (which decodes through every encoding XML permits before scanning) and migrated generate_tm7.py, generate_tb7.py, and populate_tm7_threats.py onto it, but this reader was left on the pre-fix check. A UTF-16-encoded TM7 with a DOCTYPE/ENTITY bypasses this guard; if defusedxml is unavailable, ET.fromstring resolves it (XXE).
Suggested fix:
import tm7_threat_contract
def _parse_xml(path: Path) -> ET.Element:
try:
return tm7_threat_contract.parse_hardened_xml_bytes(path.read_bytes())
except tm7_threat_contract.UnsafeXmlError as exc:
raise HarnessFailure(f"Unable to parse TM7 input: {exc}", EXIT_ERROR) from exc
Comment on lines
+1078
to
+1081
| def read_expected_surfaces(model_path: Path) -> list[SurfaceDescriptor]: | ||
| """Parse drawing surface descriptors from a TM7 model.""" | ||
| parser = DefusedET.parse if DefusedET is not None else ET.parse | ||
| tree = parser(model_path) |
Contributor
There was a problem hiding this comment.
[Security] High — no DTD/entity guard at all on the ET.parse fallback (CWE-611)
read_expected_surfaces picks DefusedET.parse if DefusedET is not None else ET.parse with no pre-check before parsing. If defusedxml is unavailable, any TM7 model handed to the validator is parsed with plain ElementTree, fully exposed to XXE-based file disclosure and entity-expansion DoS.
Suggested fix:
def read_expected_surfaces(model_path: Path) -> list[SurfaceDescriptor]:
"""Parse drawing surface descriptors from a TM7 model."""
try:
root = tm7_threat_contract.parse_hardened_xml_bytes(model_path.read_bytes())
except tm7_threat_contract.UnsafeXmlError as exc:
raise HarnessFailure(f"Unable to parse TM7 model: {exc}", EXIT_ERROR) from exc
Comment on lines
+481
to
+494
| def is_trusted_tmt_executable(path: Path) -> bool: | ||
| """Return True only for a validly signed executable from the pinned publisher. | ||
|
|
||
| Newest modification time is not a trust signal. A decoy dropped into an | ||
| otherwise allowed root would win on mtime alone, so acceptance requires a | ||
| valid Authenticode signature naming the accepted publisher. | ||
| """ | ||
| signature = _authenticode_subject(path) | ||
| if signature is None: | ||
| return False | ||
| status, subject = signature | ||
| if status != "Valid": | ||
| return False | ||
| return ACCEPTED_PUBLISHER_CN.lower() in subject.lower() |
Contributor
There was a problem hiding this comment.
[Security] Medium — unanchored substring match on Authenticode Subject (CWE-295)
is_trusted_tmt_executable checks ACCEPTED_PUBLISHER_CN.lower() in subject.lower(), an unanchored substring match against the whole raw Subject DN rather than a parsed CN comparison. A validly-signed executable whose Subject DN contains the literal substring anywhere (e.g. embedded in another RDN's value) passes even if its actual leaf CN differs.
Suggested fix (derives the expected CN from the existing ACCEPTED_PUBLISHER_CN constant instead of a second hardcoded literal):
import re
_CN_RDN = re.compile(r"(?:^|,)\s*CN=([^,]+)", re.IGNORECASE)
def is_trusted_tmt_executable(path: Path) -> bool:
signature = _authenticode_subject(path)
if signature is None:
return False
status, subject = signature
if status != "Valid":
return False
match = _CN_RDN.search(subject)
if match is None:
return False
accepted_cn = ACCEPTED_PUBLISHER_CN.split("=", 1)[-1]
return match.group(1).strip().lower() == accepted_cn.lower()
Comment on lines
+128
to
+141
| def _normalize_text(value: Any) -> str: | ||
| return "" if value is None else str(value).strip() | ||
|
|
||
|
|
||
| def _coerce_list(value: Any) -> list[Any]: | ||
| if value is None: | ||
| return [] | ||
| return value if isinstance(value, list) else [value] | ||
|
|
||
|
|
||
| def _local_name(tag: str) -> str: | ||
| return tag.rsplit("}", 1)[-1] | ||
|
|
||
|
|
Contributor
There was a problem hiding this comment.
[Standards] Medium — duplicated helpers across five scripts
_local_name, _normalize_text, _coerce_list, and _make_guid are each redefined in generate_tb7.py, generate_tm7.py, and/or populate_tm7_threats.py instead of importing the copies already living here. Since three of those files already import from tm7_threat_contract, centralizing these small helpers is low-risk.
| return payload | ||
|
|
||
|
|
||
| def validate_layout_overlay(overlay: dict[str, Any], context: OverlayContext) -> None: |
Contributor
There was a problem hiding this comment.
[Standards] Medium — oversized function (~459 lines, through line 936)
validate_layout_overlay validates schema shape, top-level keys, schema_version, per-rule geometry, identity references, and the invalidation-fingerprint block all in one function body, which is hard to unit-test or review for completeness.
Suggested fix: split into focused helpers such as _validate_overlay_shape, _validate_overlay_rules, _validate_overlay_identities, and _validate_overlay_invalidation, called in sequence from a thin orchestrator.
Comment on lines
1
to
5
| --- | ||
| name: security-planning | ||
| description: Security planning reference set for operational buckets, STRIDE analysis, standards mapping, NIST control families, and backlog scaffolding. | ||
| description: Security planning reference set for operational buckets, STRIDE analysis, standards mapping, NIST control families, backlog scaffolding, and deterministic TM7 (.tm7) plus markdown dual-output generation. | ||
| license: MIT | ||
| user-invocable: true |
Contributor
There was a problem hiding this comment.
[Standards] Low — missing compatibility frontmatter
scripts/linting/schemas/skill-frontmatter.schema.json defines compatibility as an established optional field, and 15 other skills already set it. This skill's body documents that the native TMT feedback-loop workflow requires a Windows desktop session and UI Automation access, while the rest of the skill (STRIDE analysis, portable .tm7/markdown generation) is OS-independent — worth surfacing in frontmatter, e.g. compatibility: Windows-only for the native TMT validation/feedback workflow (TM7 generation and markdown output are OS-independent).
Chris Montazer (rezatnoMsirhC)
approved these changes
Aug 4, 2026
Chris Montazer (rezatnoMsirhC)
left a comment
Chris Montazer (rezatnoMsirhC)
left a comment
Contributor
There was a problem hiding this comment.
LGTM aside from the points mentioned by Katrien De Graeve (@katriendg)
- confine every evidence path and route all sinks through redaction - refuse screenshot capture without window isolation; fail closed in strict mode - map every feedback stop reason explicitly; add harness-error fallback - serialize ET namespace mutation behind one locked boundary - reconcile security model, exit codes, and shipped docs with the runtime 🔒 - Generated by Copilot
…g-tm7-generation # Conflicts: # collections/hve-core-all.collection.md # collections/project-planning.collection.md # collections/security.collection.md # docs/reference/skills/README.md # plugins/hve-core-all/README.md # plugins/project-planning/README.md # plugins/security/README.md
- declare Windows and pinned TMT prerequisites on the security-planning skill - regenerate the skill index entry and corrected skill count 🔒 - Generated by Copilot
| raise RuntimeError("write failed") | ||
|
|
||
| # Assert | ||
| assert dict(ET._namespace_map) == caller_namespace_registry |
| try: | ||
| for _ in range(25): | ||
| outputs.append(_serialize_probe_root()) | ||
| except BaseException as exc: # pragma: no cover - reported below |
| from typing import Any | ||
| from xml.etree import ElementTree as ET | ||
|
|
||
| import tm7_threat_contract |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Description
Adds deterministic Microsoft Threat Modeling Tool (
.tm7) generation and a native validation harness to thesecurity-planningskill, so a threat model authored as a declarative spec becomes a model TMT can open, laid out legibly, without hand-building diagrams.The skill previously produced threat-model content as prose. It could not emit a
.tm7that opens cleanly, lays out readably, and can be validated without a human driving the tool by hand.What lands:
generate_tm7.pybuilds a.tm7from a YAML spec: one diagram surface per scope, trust-boundary rectangles containing their nodes, and data flows between them.generate_markdown.pyrenders the same spec as a markdown report.generate_tb7.pyemits a template.validate_tm7_with_tmt.pydrives native TMT through UI Automation, captures per-surface geometry metrics and pane-scoped screenshots, and writes a redacted evidence bundle with a schema-versioned manifest. Windows-only and opt-in; portable generation does not require TMT.approval_state: pending; no runtime path promotes them to approved.docs/planning/threat-models/hve-core-comprehensive.yamlis hve-core's own spec, previously living only in gitignored working state. Generated.tm7and markdown outputs are build artifacts and are deliberately not committed.Agent and instruction changes route the capability: the security planner and reviewer gain a TM7 workflow requiring explicit human confirmation before a generated model is treated as authored, plus an operator hands-off contract for the UI automation run.
identity.instructions.mdestablishes the spec as source of truth with markdown rendered from it, so the model is edited at the spec and re-rendered rather than edited in the rendered artifact.Related Issue(s)
Closes #2567
Type of Change
Select all that apply:
Code & Documentation:
Infrastructure & Configuration:
AI Artifacts:
hve-builderand addressed all actionable findings.github/instructions/*.instructions.md).github/prompts/*.prompt.md).github/agents/*.agent.md).github/skills/*/SKILL.md).github/hooks/*/*.json)evals/)Other:
.ps1,.sh,.py)Sample Prompts (for AI Artifact Contributions)
User Request:
Execution Flow:
security-planningskill and readsreferences/tm7-generation.mdfor the input schema and generation modes.generate_tm7.pyreads the spec, resolves a template profile, builds the model, and applies layout: nodes sized to their text, placed inside their owning trust boundary, spaced by a gutter proportional to node size.Output Artifacts:
A
.tm7file (MicrosoftSerializableModelDataDataContract XML). First lines of a generated model:A native harness run additionally produces an evidence bundle:
Success Indicators:
The model opens in TMT without template-upgrade or repair prompts. Nodes render inside their trust boundaries. Regenerating from an unchanged spec produces byte-identical output. The harness exits 0 with
automated-ready-pending-humanandgate_failure_count: 0.Testing
uv run pytest tests/test_generate_tm7.py— 136 tests, zero failures. Covers containment (nodes stay inside their owning zone), determinism under input reordering, node overlap, rank ordering, and lane separation across 8 layout archetypes.Deserialize-Tm7.ps1checks generated models against TMT assemblies for round-trip fidelity.fuzz_harness.pyprovides an Atheris polyglot entry point for OSSF Scorecard fuzzing.uv run ruff check scripts tests— clean.Checklist
Required Checks
AI Artifact Contributions
hve-builderreview mode to review contributionhve-builderreviewRequired Local Checks
The following local-safe validation commands must pass before merging:
npm run validate:localnpm run validate:docsnpm run spell-checknpm run lint:md-linksSecurity Considerations
This PR adds an executable runtime to a skill that previously shipped only reference material, so the security surface is new.
SECURITY.mdin the skill carries the full STRIDE model over three trust buckets (TMT process automation and UI Automation, local screenshots and evidence, overlay manifest and path handling) with risk ratings and eight open enterprise-readiness gaps.Observed properties of the runtime:
<!DOCTYPEand<!ENTITYbefore parse and prefersdefusedxml, closing the XXE path on specs, models, and templates.yaml.safe_loadandjson.loadonly. Noeval,exec,pickle, or dynamic import.approval_state: pendingand are never auto-promoted, so visual scores stay advisory rather than becoming silent approval.Dependencies added:
pyyaml==6.0.3anddefusedxml>=0.7at runtime;pytestandrufffor development;atherisisolated in afuzzgroup because macOS wheels are unavailable;pillowandpywinautoin a Windows-only group for UI Automation.uv.lockis committed so Dependabot resolves through the.github/skills/**glob.THIRD-PARTY-NOTICESrecords MIT attribution for the bundled Microsoft threat-modeling templates.Additional Notes
The committed threat-model spec is marked DRAFT and has not had human security review.
docs/planning/threat-models/README.mdstates this explicitly so neither the spec nor anything generated from it is mistaken for a reviewed artifact.Generated models are intentionally not committed. A
.tm7for this repository is roughly 1.8 MB of XML, and any layout change rewrites geometry across every surface, so committing outputs would produce large unreviewable diffs on one-line source changes. The spec is the versioned source;README.mddocuments the regeneration command.The native harness is Windows-only and opt-in. It requires TMT 7.3.51110.1 and takes exclusive control of mouse and keyboard for the duration of a run. Generation and markdown rendering are portable and require neither.
Known open items are tracked in the skill's
SECURITY.mdrather than left implicit. The ones most worth a reviewer's attention:Bundle.path()does not reject..components, which is unreachable from current callers because they all pass hardcoded relative paths but could be reintroduced by a refactor; evidence bundles carry no integrity signature, so post-run tampering is not detectable without a comparison run; and the pinned TMT version is a module constant, so a tool upgrade requires a code change or the diagnostic override.Two markdown link-check failures appear in
npm run validate:localand are pre-existing onmain, in files this PR does not touch:https://www.omg.org/spec/DMN/1.4/(HTTP 522) inrequirements-authorandhttps://dev.azure.com(unreachable) inextension/PACKAGING.md. Verified by reproducing both on a clean checkout oforigin/main.