feat: Add security threat model - #328
Open
turbobobbytraykov wants to merge 2 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds initial security documentation artifacts for the Ignite UI for Blazor repo: a draft STRIDE-based threat model for IgniteUI.Blazor.Lite / IgniteUI.Blazor.Templates, plus a repeatable security review record template intended to be copied per release.
Changes:
- Added
docs/security/threat-model.mddocumenting scope, trust boundaries, identified threats, and residual/accepted risks. - Added
docs/security/review-template.mdproviding a structured review checklist and findings register to accompany the threat model during release reviews.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| docs/security/threat-model.md | New threat model document covering architecture, assumptions, threats, and accepted residual risks for Lite/Templates. |
| docs/security/review-template.md | New template for recording a security review outcome and tracking threat dispositions per package/version. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| | ID | Threat | STRIDE | Sev | Status | | ||
| |---|---|---|---|---| | ||
| | **TM-IX-01** | `WebCallback` is a **public** class whose `[JSInvokable]` methods (`OnReady`, `OnInvokeReturn`, `OnRaiseEvent`, `AdjustDynamicContent`, `AdjustDynamicContentBatch`) all take a **client-supplied `containerId`** used as a key into a process-wide `_controlsMap`. A caller that reaches the reference can address *any* registered control in the circuit, not only the one it legitimately owns — event raising and dynamic-content mutation can be driven cross-instance. This is the largest single item in the model. | S, T, E | **High** | **Open** | |
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
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.
Closes #344
Description
Adds two documents — the first threat model for
IgniteUI.Blazor.LiteandIgniteUI.Blazor.Templates, plus the template for the security review that follows it:Documentation only; no code changes.
Motivation / Context
This PR establishes a maintained threat model and a repeatable security-review record for both packages. The dated review and findings register follow after the model is merged and the High-severity items are resolved — see Next steps.
The model is therefore structured against Microsoft's own Blazor threat-mitigation guidance, with the document shape taken from the CNCF/OpenSSF self-assessment format. It uses the same threat IDs as the model in
IgniteUI.Blazor.GridLite, so the two can be reviewed and maintained together.How to read it
STRIDE applied to three trust boundaries — server→client serialization (TB1), the managed→WASM unmarshalled path (TB1b, unique to this repo), and client→server JS interop (TB2). Every threat has an ID, a severity and a status. Severity is the residual severity after the stated assumptions, which matters: most interop threats presuppose attacker script in the page, and per Microsoft's guidance an XSS-compromised client can already forge interop calls. Our obligation is to not cause XSS and to not widen the blast radius — the findings below are judged on that basis.
The two packages are modelled separately because their risk is different in kind:
Liteis a runtime interop surface;Templatesis a supply-chain and secure-defaults surface.One thing worth flagging because it looks alarming and is not: Microsoft's guidance names
AddMarkupContentas an XSS vector, andDynamicContentHoldercalls it. Every call site was checked — all pass static whitespace literals, never user data. Recorded as Verified — no finding so reviewers do not have to re-derive it.What it found
IgniteUI.Blazor.LiteWebCallbackis public and its per-control[JSInvokable]methods key off a client-suppliedcontainerIdinto the scoped runtime's control map — a caller reaching the reference can drive event raising and dynamic-content mutation on another registered control in the same scopeRuntimeHelperreflection-discoversInvokeUnmarshalled, compiles a delegate, and calls it fromunsafecode passing raw WASM-heap pointers (AllowUnsafeBlocks=true). A layout mismatch is memory corruption, not an exceptionnpm auditgate in CIunsafeHTMLusage was found in this repository, but the rendering behavior of the bundledigniteui-webcomponentsdependency still needs confirmationOnInvokeReturntakes untypedobject;AdjustDynamicContentBatchdeserializes a client-supplied batch string and drives render-tree mutation${{ env.BUILD_CONFIGURATION }}is referenced in the release signing/validation steps but never defined — it expands to empty, so the integrity gate scanssrc/bin/instead ofsrc/bin/Release/. It only works because the recursive glob still reaches the Release outputIgniteUI.Blazor.TemplatesNoDefaultExcludes=trueplus a broadContent Includeexcluding onlybin/objpacks everything else in the tree — dotfiles,.env, editor state — into the shipped package<Version>0.0.1</Version>hard-coded rather than driven by the release tagExisting controls include CodeQL default setup, SHA-pinned release actions, Azure OIDC, NuGet Trusted Publishing, signing with verification gates,
package-manager-cache: falsein release builds, deterministic builds, central package management, Dependabot with a cooldown, and a completeSECURITY.mdwith private reporting and stated SLAs.What we need from you to approve
You do not need threat-modelling experience. Please check four things:
igniteui-webcomponentsrendering path needs to confirm whetherunsafeHTML(or equivalent) is used for any bound value. No such use was found in this repository, but the dependency's rendering path still blocks sign-off.Leave the
<!-- TODO(maintainer) -->placeholders (commit SHA, owner, approver names) — they are filled in at sign-off, not at merge.Next steps (after merge)
WebCallback: reduce its public surface and bind callbacks to the instance that owns them rather than to a client-supplied key. This is a code change and the largest item in the model.BUILD_CONFIGURATIONand fail the signature gate on an empty result set.docs/security/review-<package>-<version>.md.References
Primary — Microsoft
AddMarkupContentXSS warningSECURITY.mdand microsoft/fluentui-blazorSECURITY.md— Microsoft's own Blazor library disclosure policy; note that neither repository publishes a threat modelDocument format
security-insights.yml; a candidate follow-upSupply chain / build integrity
dotnet newcustom templates — for the template packaging findingsType of Change (check all that apply):
Component(s) / Area(s) Affected:
How Has This Been Tested?
Test Configuration:
Screenshots / Recordings
Checklist:
README.MDCHANGELOG.MDupdates for newly added functionalityCloses #