ASD STIG / NIST 800-53 review of untrusted-input boundaries with remediations

[erikrozi]

Closes N/A (security review; no linked issue)

Describe your changes:

Source-code review of Open MCT's untrusted-input and data boundaries against the DISA ASD STIG V6R4 (2025-09-09) and NIST SP 800-53 Rev. 5, plus remediation of the highest-value findings with Karma specs. The full assessment, including every boundary's explicit outcome and the deployment evidence an assessor would need, is in docs/security/asd-stig-nist-800-53-review.md. This is a code review, not a certification or attestation of compliance.

Findings already addressed by #10 (Comps mathjs), #11 (Web Page iframe sandbox / URL allowlist), #12 (npm registry signatures) and #13 (CSV formula injection) are excluded and not duplicated.

Findings table

Severity is the ASD STIG category of the cited rule. Line numbers refer to this branch.

ID	File:line	Description	CWE	ASD STIG V6R4	NIST 800-53 r5	Sev	Outcome
F-01	importFromJSONAction/ImportFromJSONAction.js:87	Import tree parsed and persisted with only a __proto__ filter; constructor/prototype keys, identifier/key mismatches, non-string types, bad composition/condition-set references and unknown roots accepted	CWE-1321, CWE-20	V-222606	SI-10	II	not-satisfied -> remediated
F-02	ImportFromJSONAction.js:106,452	Import failures surfaced parser/provider text to operator	CWE-209	V-222610	SI-11	II	not-satisfied -> remediated
F-03	ImportFromJSONAction.js:348-350	Objects saved while walking the tree; later validation failure left partial state	CWE-20	V-222609	SI-10(3)	I	not-satisfied -> remediated
F-04	exportAsJSONAction/ExportAsJSONAction.js:102-107	Export failure rethrew raw errors to UI	CWE-209	V-222610	SI-11	II	not-satisfied -> remediated
F-05	notebook/components/NotebookEntry.vue:460-479	Sanitizer schema allowed data:/protocol-relative links; link allowlist matched hostname suffixes; hand-built anchor did not escape text/href	CWE-79	V-222602	SI-10	I	not-satisfied -> remediated
F-06	NotebookEntry.vue:571, NotebookComponent.vue:676, notebook-image.js:72	Raw error interpolated into operator notification	CWE-209	V-222610	SI-11	II	not-satisfied -> remediated
F-07	persistence/couch/CouchObjectProvider.js:233-285	Network, HTTP and malformed-response failures propagated raw fetch/CouchDB text	CWE-209, CWE-755	V-222610, V-222656	SI-11, SA-15(5)	II	not-satisfied -> remediated
F-08	localStorage/LocalStorageObjectProvider.js:37-105	Browser storage / JSON.parse exceptions thrown raw to callers	CWE-209, CWE-755	V-222610, V-222656	SI-11, SA-15(5)	II	not-satisfied -> remediated
F-09	utils/textHighlight/TextHighlight.vue:24,63-65	Highlight term inserted into v-html markup unescaped and used as an unescaped regex	CWE-79, CWE-1333	V-222602	SI-10	I	not-satisfied -> remediated
F-10	13 innerHTML sites (listed in doc)	Empty-string or static-template assignments, no user data	CWE-79	V-222602	SI-10	I	satisfied
F-11	formActions/CreateAction.js:94	Error saving objects: ${err} shown to operator	CWE-209	V-222610	SI-11	II	not-satisfied -> remediated
F-12	ui/layout/AboutDialog.vue:30	v-html of deployer Branding.aboutHtml (configuration, not user data)	CWE-79	V-222602	SI-10	I	satisfied
F-13	UserAPI.js:135, FaultManagementAPI.js:122-171, import/export actions, notebook-entries.js:242-356, NotebookComponent.vue:586	No audit records for import, export, role change, notebook entry create/delete, fault ack/shelve	CWE-778	V-222471, V-222472, V-222476, V-222477	AU-2, AU-3, AU-12	II	not-satisfied -> remediated
F-14	URLIndicatorPlugin/URLIndicator.js:78,100	Polls deployer-configured URL; scheme is configuration	CWE-319	V-222596	SC-8	I	needs-input
F-15	persistence/couch/plugin.js:36-68, CouchObjectProvider.js:218	CouchDB TLS termination/version not visible in source	CWE-319	V-222596, V-222597	SC-8, SC-13	I	needs-input
F-16	CouchObjectProvider.js:218	No embedded credentials (satisfied, V-222642); CouchDB authn/authz is deployment	CWE-306	V-222522, V-222642	IA-2, IA-5(7), AC-3	I	needs-input
F-17	example/exampleUser/ExampleUserProvider.js:96-240, userIndicator, operatorStatus, api/user	Example provider is in-memory with auto-login; not production authentication. Status/poll inputs use text interpolation (satisfied for XSS)	CWE-287, CWE-285	V-222522, V-222425	IA-2, AC-3	I	needs-input; example provider not-applicable
F-18	LocalStorageObjectProvider.js, api/user/StoragePersistence.js:27-33	Objects and active role stored unencrypted in localStorage; CouchDB at-rest is server-side	CWE-312	V-222587, V-222588	SC-28	II/I	needs-input
F-19	deployment (proxy / IdP)	Session IDs, idle timeout, logoff, cookie flags not in this client	CWE-613	V-222577, V-222389, V-222391, V-222388	AC-12, SC-23	I/II	needs-input
F-20	api/audit/AuditLogger.js	Audit records in-process only; centralized forwarding is deployment	CWE-778	V-222475	AU-4, AU-6, AU-9	II	needs-input
F-21	condition/utils/operations.js:45, summaryWidget/src/ConditionEvaluator.js:344-413	User-authored criteria evaluated via fixed operation table; no eval/new Function	CWE-94	V-222609, V-222604	SI-10, CM-7	I	satisfied
F-22	hyperlink/HyperlinkLayout.vue:50	href passed through @braintree/sanitize-url	CWE-79	V-222602	SI-10	I	satisfied
F-23	notebook/components/NotebookEmbed.vue:31, notebook-image.js	Snapshot images bound via :src from app-generated captures	CWE-79	V-222602	SI-10	I	satisfied
F-24	NotebookEntry.vue:684	Entry text stripped to plain text before storage	CWE-79	V-222606	SI-10	II	satisfied
F-25	api/objects/ObjectAPI.js:441	Conflict message includes only object key-string	CWE-209	V-222600	SI-11	II	satisfied

Counts: 11 not-satisfied -> remediated; 7 satisfied; 7 needs-input; 1 not-applicable; 0 not-satisfied left open.

Remediated in this PR

	Change	Specs
R1	importValidation.js (new): rejects __proto__/constructor/prototype at any depth, validates root, identifier/key-string consistency, type/name/location, composition and condition-set references; ImportFromJSONAction validates the whole tree before the first save(), shows fixed messages, logs diagnostics, and emits import audit records	importValidationSpec.js, ImportFromJSONActionSpec.js
R2	NotebookEntry.vue: sanitize-html schema restricted to http/https/mailto, no protocol-relative URLs, enumerated attributes; link renderer requires http(s):, exact host or dot-delimited subdomain, escapes text and href, adds rel="noopener noreferrer". TextHighlight.vue: HTML- and regex-escaped term, escaped class, $1 reinsertion	NotebookEntrySpec.js (new), TextHighlightSpec.js (new)
R3	PersistenceError (new, exposed as openmct.objects.errors.Persistence); Couch and LocalStorage providers normalize failures to it with raw details in console.error; Conflict preserved; generic messages in export, notebook image and CreateAction paths	couch/pluginSpec.js, localStorage/pluginSpec.js, ExportAsJSONActionSpec.js, CreateActionSpec.js
R4	AuditLogger (new) registered as openmct.audit; records {id, source, timestamp, action, outcome, actor{id,username,role}, target, details} to subscribers via addProvider({ record }); hooks in UserAPI.setActiveRole, FaultManagementAPI.acknowledgeFault/shelveFault, import/export actions, notebook entry create/delete. No external sink	AuditLoggerSpec.js, UserAPISpec.js, FaultManagementAPISpec.js, notebook-entriesSpec.js

Root cause for the error-message findings (F-02/04/06/07/08/11): the raw-error-to-operator pattern was repeated at six sites with no central rule. R3 centralizes provider failures in PersistenceError so any caller displaying error.message from the object API now shows generic text; the UI-level sites were fixed individually. Audit hooks were placed at the API layer where one exists (UserAPI, FaultManagementAPI) so every caller is covered.

Documented / needs-input

Evidence an ISSM/assessor must supply (details in the doc): CouchDB TLS configuration and version policy (F-15); URL indicator target scheme (F-14); CouchDB authentication and _security role model plus the production UserProvider/IdP/MFA (F-16, F-17); proxy/IdP session timeout, logoff and cookie flags (F-19); data-sensitivity determination for browser localStorage and CouchDB host encryption (F-18); reverse-proxy security headers (CSP, HSTS, X-Content-Type-Options, frame-ancestors); an openmct.audit provider forwarding to the site log store (F-20).

Test baseline comparison

Run	npm run lint	npm test	Failing specs
master (a4aae41af)	pass	6 FAILED, 969 SUCCESS	Object API Search x4, Image Exporter x1, URLIndicator default icon class x1
this branch (7c8deeb07)	pass (eslint js+vue, cspell)	6 FAILED, 1048 SUCCESS	same 6
this branch (final)	pass (eslint js+vue, cspell)	0 FAILED, 1061 SUCCESS (67 skipped)	none

The six baseline specs are environment-sensitive, are not touched by this PR, and did not reproduce on the final run (CI unit-test also passes); +92 specs, no regressions. Notebook e2e link tests run locally via npm run test:e2e:ci -- tests/functional/plugins/notebook/notebook.e2e.spec.js: 17 passed / 22 skipped (upstream fixme), no e2e test edited. cspell entries added: STIG, DISA, ISSM, remediations, rethrew, notexample, Deployers, stig.

Original prompt

Do an Application Security & Development STIG and NIST SP 800-53 Rev. 5 review of Open MCT's untrusted-input boundaries (import/export JSON, notebook entries, URL plugins, CouchDB/localStorage persistence, user/role plugins, expression evaluation). Produce a findings table with file:line, CWE, ASD STIG rule ID, NIST control, severity and an explicit outcome for every boundary (satisfied / not-satisfied / needs-input / not-applicable). Then remediate the top findings in one PR with unit tests: schema validation and prototype-pollution protection on import, XSS sanitization on notebook sinks, generic error handling on persistence failures, and a structured audit-event hook for operator actions. Keep lint and the test suite green and show a baseline comparison.

All Submissions:

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?
• Is this a notable change that will require a special callout in the release notes? For example, will this break compatibility with existing APIs or projects that consume these plugins? Yes: openmct.audit is a new API; persistence providers now reject with openmct.objects.errors.Persistence instead of raw fetch/browser errors; Import-from-JSON now rejects trees that previously imported partially.

Author Checklist

• Changes address original issue?
• Tests included and/or updated with changes?
• Has this been smoke tested? (Karma unit specs only; no browser smoke test performed)
• Have you associated this PR with a type: label? Note: this is not necessarily the same as the original issue.
• Have you associated a milestone with this PR? Note: leave blank if unsure.
• Testing instructions included in associated issue OR is this a dependency/testcase change? (npm run lint && npm test)

Reviewer Checklist

• Changes appear to address issue?
• Reviewer has tested changes by following the provided instructions?
• Changes appear not to be breaking changes?
• Appropriate automated tests included?
• Code style and in-line documentation are appropriate?

Link to Devin session: https://app.devin.ai/sessions/9a388bf2196f49ee9af1542b613e2510
Open in Devin Desktop: https://app.devin.ai/desktop/session/9a388bf2196f49ee9af1542b613e2510?variant=devin
Requested by: @erikrozi

---

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR that start with 'DevinAI' or '@devin'.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring

---

Original prompt from Devin Bot

Repository: COG-GTM/openmct (fork of NASA Open MCT, branch master). Clone it, read README.md, CONTRIBUTING.md, TESTING.md, SECURITY.md, and AGENTS.md if present. Read the already-open security PRs on this fork so you do not duplicate them: #10 (Comps SharedWorker mathjs hardening), #11 (Web Page iframe sandbox / URL allowlist), #12 (npm registry signatures), #13 (CSV formula injection). Your work must cover DIFFERENT findings.

You have organization knowledge titled "Federal Security Compliance Knowledge" (STIG V-2206xx ↔ NIST 800-53 mappings, secure coding requirements). Use it as your control vocabulary. The repo COG-GTM/federal_security_compliance also contains devin/knowledge.md, devin/playbook.md and a NIST 800-53 scan playbook — read them before you start.

#``# Task

Perform an Application Security and Development (ASD) STIG + NIST SP 800-53 Rev. 5 code review of Open MCT's untrusted-input and data-boundary surfaces, then remediate the highest-value findings in one PR with tests.

Part A — Assessment (write it as docs/security/asd-stig-nist-800-53-review.md in the repo, and also summarize in the PR description):

1. Enumerate the trust boundaries: Import-from-JSON action (src/plugins/importFromJSONAction), Export-as-JSON, Notebook entries and embeds (src/plugins/notebook), Hyperlink / URL indicator plugins, CouchDB persistence provider (src/plugins/persistence/couch), LocalStorage persistence, the Example User / operator-status / role model (example/exampleUser, src/plugins/userIndicator, src/plugins/operatorStatus), Condition Sets / Summary Widgets that evaluate user-authored expressions, and any innerHTML/v-html usage.
2. For each boundary, record findings in a table: ID, file:line, description, CWE, ASD STIG rule (use real STIG rule IDs such as V-222602 input validation, V-222609 XSS, V-222612 / V-222601 error handling, V-222604 audit records, V-222577 authenticated sessions, V-222596 encrypted transport, etc. — look up the exact cur... (3958 chars truncated...)

[devin-ai-integration]

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

Devin Review found 5 potential issues.

[devin-ai-integration]

🔍 Senior API approval required

The new public openmct.audit API requires senior-developer approval under CONTRIBUTING.md. Confirm that approval before merge.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

Acknowledged. openmct.audit is a new public API and per CONTRIBUTING.md needs senior-developer approval before merge; this PR is intentionally left unmerged for that review. The API surface is small (record, addProvider, removeProvider, hasProviders, EventEmitter on/off/once) and documented in API.md.

[devin-ai-integration]

Browser evidence: import validation, Notebook XSS sanitization, audit records

Driven through the real UI at http://localhost:8080 (LocalStorage persistence, npm start) on this branch, with ExampleUser (operator-one, roles flight/driver) and ExampleFaultSource enabled temporarily for the role-change and fault scenarios (reverted afterwards; no index.html diff).

Annotated recording (2 min): security-browser-tests-edited.mp4 · full write-up: testing-report.md

Scenarios

#	Scenario	Result
1	Import __proto__ payload	Generic "import rejected" notification; reasons only in console; no object created, no LocalStorage change, ({}).polluted === undefined
2	Import constructor.prototype payload	Same as 1
3	Import identifier / key-string mismatch	Rejected generically; no persistence
4	Import referencing a composition child absent from the payload	Accepted (see note below)
5	Export folder + child, re-import into another folder	Roundtrip succeeds; both objects present
6	Notebook entry: <img src=x onerror=alert(1)> <script>…</script> [click](javascript:…) **bold**	No alert; rendered as <p> click ok bad <strong>bold</strong></p>; no img/script/a elements; https:, javascript: and data: links become plain text when no allowlist is configured
7	Search highlighting with <b>x and ( queries	Literal text, no markup injection, no regex error
8	Audit records for export, import, notebook entry create/delete, role change (flight → driver), fault acknowledge, fault shelve	One record each with id, source, timestamp (ISO UTC), action, outcome, actor{id,username,role}, details
9	Storage.setItem forced to throw QuotaExceededError internal-path-secret	Operator sees only "Error saving objects"; the secret string appears in console diagnostics only

Rejected imports (generic message, clean prototype)	Notebook payload rendered inert

Audit records + required-field checks	Generic save error shown to operator

Detailed error confined to console	Export/import roundtrip

Findings from the browser run and their disposition

1. Rejected imports emitted no import/failure audit record when the file was rejected by the form validator (before OK is enabled). Fixed in 887b66c: the form validator now routes through _rejectImport(), so the generic notification and the failure audit record are emitted on the same path as save-time rejections. Covered by a new Karma spec (ImportFromJSONActionSpec.js, "rejects invalid files in the form validator…").
2. Composition reference to an object absent from the payload was accepted (rendered as Missing: absent / Unknown Type). Kept as-is on purpose: ExportAsJSONAction#exportObject deliberately omits non-creatable children (e.g. read-only telemetry) while leaving their identifiers in the parent's composition, so a valid export can legitimately contain such references. Rejecting them would break real roundtrips; the reference is validated for shape and is never dereferenced as code. Documented in docs/security/asd-stig-nist-800-53-review.md (F-01).

CI follow-ups in this push (887b66c)

• e2e-ci shard 2/4 (Notebook link tests, regression from R2): root cause was rendering url.href (canonicalised, e.g. trailing /, empty-query normalisation) instead of the original href in the generated anchor, so the upstream selectors that match on the literal text no longer found the anchor. The allowlist rule (exact host or dot-delimited subdomain) was not the cause — google.com correctly admits bing.google.com and www.google.com. Fix: validate via new URL(href) but render the escaped original string. Local run of npm run test:e2e:ci -- tests/functional/plugins/notebook/notebook.e2e.spec.js: 17 passed / 22 skipped (skipped = upstream test.fixme), including lines 429, 480, 494 and 517. No e2e test was edited.
• visual-a11y-ci (1) "main view" colour-contrast (#050505 on #000000): not caused by this branch. Evidence: the same head commit 7c8deeb ran the job four times (runs 34164649617, 34164721574, 34164927696 → pass; 34164955621 → fail), and the identical violation appeared on an unrelated branch on the same base (run 34163719018, devin/1788814796-flight-test-telemetry-plugin, job 101871007996). The flagged elements are the Inspector pane labels while the pane is collapsing, i.e. an animation-timing flake in the upstream test.
• e2e-couchdb: pre-existing displayLayout @couchdb failure, identical on Add html and text-summary coverage reporters to karma config #15/Add Flight Test Telemetry example plugin (PCM, MIL-STD-1553, TSPI, faults, IRIG 106 Ch10 adapter) #16; not addressed.
• Also in this push (from the automated review): bounded import traversal (64 nesting levels / 20 collected errors) and isolation of throwing or rejecting audit listeners/providers. Lint clean; npm test → 1061 passed / 0 failed / 67 skipped (base: 969 passed).

Written by Devin

[devin-ai-integration]

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

Devin Review found 9 new potential issues.

[devin-ai-integration]

🔍 Required issue link is missing

The contribution rules require every pull request to link an issue. The description uses “Closes N/A,” leaving no required traceability record.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

There is no tracking issue on this fork for this review; the work was requested directly, so "Closes N/A" is deliberate. If a tracking issue is opened I will link it in the description.