From 6007b9ecc577bd2babe1652ef13c66a5e81f5bcd Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Thu, 28 May 2026 14:10:03 +0200 Subject: [PATCH] fix(rules,web): pin rule template interpolation to the required-condition event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex review (P2): ANALYZE-APP-013's `failed_app_name` + `detection_hresult` optional conditions scanned all app_install_failed events independently of the required condition. In a session with multiple app failures the template could interpolate {{appName}} / {{errorCode}} from a different event than the one that matched `failureType == esp_apps_detection_failure`. Two coordinated changes: 1. `interpolateRuleTemplate` gains an "auto-field" resolution tier between the explicit byField path and the bySignal fallback. The backend's `RuleEngine.AddDataFieldsToEvidence` already injects a whitelist of identifying fields (appId, appName, errorPatternId, errorCode, exitCode, status) onto every matched-condition evidence dict — the interpolator now reads those, pinning to the FIRST matched condition (= the required condition) so a rule with a single required match resolves all four tokens against the same source event. 2. ANALYZE-APP-013 is trimmed to a single required condition; the redundant optional conditions for appName / errorCode are removed because the auto-field tier already exposes them from the required match. Same pattern applied to ANALYZE-ENRL-001's `failed_app` condition. Coverage: 4 new vitest cases (multi-failure correct interpolation, first- condition-wins shadowing, explicit-field precedence, internal-field leakage guard). 413/413 web tests. Co-Authored-By: Claude Opus 4.7 (1M context) --- rules/analyze/ANALYZE-APP-013.json | 18 --- rules/analyze/ANALYZE-ENRL-001.json | 11 +- rules/dist/analyze-rules.json | 29 +---- .../__tests__/interpolateRuleTemplate.test.ts | 107 ++++++++++++++++++ .../lib/interpolateRuleTemplate.ts | 27 ++++- 5 files changed, 134 insertions(+), 58 deletions(-) diff --git a/rules/analyze/ANALYZE-APP-013.json b/rules/analyze/ANALYZE-APP-013.json index d5771c73..bba4c66e 100644 --- a/rules/analyze/ANALYZE-APP-013.json +++ b/rules/analyze/ANALYZE-APP-013.json @@ -16,24 +16,6 @@ "operator": "equals", "value": "esp_apps_detection_failure", "required": true - }, - { - "signal": "failed_app_name", - "source": "event_type", - "eventType": "app_install_failed", - "dataField": "appName", - "operator": "exists", - "value": "", - "required": false - }, - { - "signal": "detection_hresult", - "source": "event_type", - "eventType": "app_install_failed", - "dataField": "errorCode", - "operator": "exists", - "value": "", - "required": false } ], "explanation": "App **{{appName}}** failed during the Enrollment Status Page with HRESULT **{{errorCode}}** — *Application not detected after installation completed successfully*. The installer reported success but the Intune detection rule (file/registry/script) did not find the app afterwards, so the ESP Apps subcategory failed and the device's enrollment terminated.\n\nThis is a confirmed detection-rule mismatch, NOT an install timeout — the install itself most likely completed (or the installer exited 0). Common root causes:\n\n- The detection rule looks for a registry/file path that the installer doesn't create (wrong path, wrong bitness — e.g. checking HKLM\\SOFTWARE\\... on a 32-bit installer that lands under WOW6432Node).\n- The installer writes the detection artefact AFTER the ESP's detection probe runs (e.g. async post-install task, requires reboot).\n- The detection script returns 0 with no stdout (Intune treats empty stdout as \"not detected\").\n- The installer wrote files to a user profile but the rule looks in System scope (or vice-versa).", diff --git a/rules/analyze/ANALYZE-ENRL-001.json b/rules/analyze/ANALYZE-ENRL-001.json index 7721578c..1326840a 100644 --- a/rules/analyze/ANALYZE-ENRL-001.json +++ b/rules/analyze/ANALYZE-ENRL-001.json @@ -42,21 +42,12 @@ "operator": "exists", "value": "", "required": false - }, - { - "signal": "failed_app", - "source": "event_type", - "eventType": "app_install_failed", - "dataField": "appName", - "operator": "exists", - "value": "", - "required": false } ], "confidenceFactors": [ { "signal": "app_failed", "condition": "exists", "weight": 5 } ], - "explanation": "The Autopilot enrollment has explicitly failed. The most specific failure information is carried by the events captured during this session — review the `enrollment_failed` event payload and the more targeted rule results (ANALYZE-APP-* / ANALYZE-ESP-*) that fired alongside this one for the actionable root cause.\n\nKey facts extracted from the timeline:\n\n- **Failure reason:** `{{reason}}`\n- **ESP failed subcategory:** `{{failedSubcategory}}`\n- **ESP HRESULT:** `{{errorCode}}`\n- **First failed app:** `{{appName}}`\n\n(Fields shown as `{{…}}` were not present in this session — e.g. a non-ESP failure path has no failed-subcategory, and a successful pre-ESP stage has no failed app.)", + "explanation": "The Autopilot enrollment has explicitly failed. The most specific failure information is carried by the events captured during this session — review the `enrollment_failed` event payload and the more targeted rule results (ANALYZE-APP-* / ANALYZE-ESP-*) that fired alongside this one for the actionable root cause.\n\nKey facts extracted from the timeline:\n\n- **Failure reason:** `{{reason}}`\n- **ESP failed subcategory:** `{{failedSubcategory}}`\n- **ESP HRESULT:** `{{errorCode}}`\n\n(Fields shown as `{{…}}` were not present in this session — e.g. a non-ESP failure path has no failed-subcategory.)", "remediation": [ { "title": "Investigate enrollment failure", diff --git a/rules/dist/analyze-rules.json b/rules/dist/analyze-rules.json index d930ada8..44a66fcc 100644 --- a/rules/dist/analyze-rules.json +++ b/rules/dist/analyze-rules.json @@ -516,24 +516,6 @@ "operator": "equals", "value": "esp_apps_detection_failure", "required": true - }, - { - "signal": "failed_app_name", - "source": "event_type", - "eventType": "app_install_failed", - "dataField": "appName", - "operator": "exists", - "value": "", - "required": false - }, - { - "signal": "detection_hresult", - "source": "event_type", - "eventType": "app_install_failed", - "dataField": "errorCode", - "operator": "exists", - "value": "", - "required": false } ], "explanation": "App **{{appName}}** failed during the Enrollment Status Page with HRESULT **{{errorCode}}** — *Application not detected after installation completed successfully*. The installer reported success but the Intune detection rule (file/registry/script) did not find the app afterwards, so the ESP Apps subcategory failed and the device's enrollment terminated.\n\nThis is a confirmed detection-rule mismatch, NOT an install timeout — the install itself most likely completed (or the installer exited 0). Common root causes:\n\n- The detection rule looks for a registry/file path that the installer doesn't create (wrong path, wrong bitness — e.g. checking HKLM\\SOFTWARE\\... on a 32-bit installer that lands under WOW6432Node).\n- The installer writes the detection artefact AFTER the ESP's detection probe runs (e.g. async post-install task, requires reboot).\n- The detection script returns 0 with no stdout (Intune treats empty stdout as \"not detected\").\n- The installer wrote files to a user profile but the rule looks in System scope (or vice-versa).", @@ -1001,15 +983,6 @@ "operator": "exists", "value": "", "required": false - }, - { - "signal": "failed_app", - "source": "event_type", - "eventType": "app_install_failed", - "dataField": "appName", - "operator": "exists", - "value": "", - "required": false } ], "confidenceFactors": [ @@ -1019,7 +992,7 @@ "weight": 5 } ], - "explanation": "The Autopilot enrollment has explicitly failed. The most specific failure information is carried by the events captured during this session — review the `enrollment_failed` event payload and the more targeted rule results (ANALYZE-APP-* / ANALYZE-ESP-*) that fired alongside this one for the actionable root cause.\n\nKey facts extracted from the timeline:\n\n- **Failure reason:** `{{reason}}`\n- **ESP failed subcategory:** `{{failedSubcategory}}`\n- **ESP HRESULT:** `{{errorCode}}`\n- **First failed app:** `{{appName}}`\n\n(Fields shown as `{{…}}` were not present in this session — e.g. a non-ESP failure path has no failed-subcategory, and a successful pre-ESP stage has no failed app.)", + "explanation": "The Autopilot enrollment has explicitly failed. The most specific failure information is carried by the events captured during this session — review the `enrollment_failed` event payload and the more targeted rule results (ANALYZE-APP-* / ANALYZE-ESP-*) that fired alongside this one for the actionable root cause.\n\nKey facts extracted from the timeline:\n\n- **Failure reason:** `{{reason}}`\n- **ESP failed subcategory:** `{{failedSubcategory}}`\n- **ESP HRESULT:** `{{errorCode}}`\n\n(Fields shown as `{{…}}` were not present in this session — e.g. a non-ESP failure path has no failed-subcategory.)", "remediation": [ { "title": "Investigate enrollment failure", diff --git a/src/Web/autopilot-monitor-web/lib/__tests__/interpolateRuleTemplate.test.ts b/src/Web/autopilot-monitor-web/lib/__tests__/interpolateRuleTemplate.test.ts index ef96ca07..2ec018fc 100644 --- a/src/Web/autopilot-monitor-web/lib/__tests__/interpolateRuleTemplate.test.ts +++ b/src/Web/autopilot-monitor-web/lib/__tests__/interpolateRuleTemplate.test.ts @@ -71,4 +71,111 @@ describe("interpolateRuleTemplate", () => { it("does not blow up on text with no placeholders", () => { expect(interpolateRuleTemplate("plain text", matched)).toBe("plain text"); }); + + // Codex review follow-up (P2): when a rule's required condition matches an event + // that carries auto-injected whitelist fields (appName, errorCode, etc.), the + // template MUST resolve those tokens from the SAME event — even if other + // optional conditions matched different events. The interpolator pins to the + // first matched-conditions entry so the rule's required (= first) condition + // wins. + it("ANALYZE-APP-013 multi-failure: interpolation uses the required-condition event, not the first failed app", () => { + const matchedConditions = { + // Required condition: matched the Office app with HRESULT 0x87D1041C. + esp_apps_detection_failure: { + eventId: "evt-office", + sequence: 168, + timestamp: "2026-05-28T10:41:08Z", + eventType: "app_install_failed", + field: "failureType", + value: "esp_apps_detection_failure", + // AddDataFieldsToEvidence auto-injects these from the SAME event: + appId: "f9026516-dbec-47cd-ae4e-ec2312a1303c", + appName: "Microsoft Apps for Enterprise - 64Bit", + errorPatternId: "esp_apps_detection_failure", + errorCode: "0x87d1041c", + }, + }; + + const explanation = + "App **{{appName}}** failed during the Enrollment Status Page with HRESULT **{{errorCode}}**."; + const out = interpolateRuleTemplate(explanation, matchedConditions); + expect(out).toBe( + "App **Microsoft Apps for Enterprise - 64Bit** failed during the Enrollment Status Page with HRESULT **0x87d1041c**." + ); + }); + + it("auto-field resolution pins to the first matched condition (no cross-event drift)", () => { + // Order matters: the FIRST matched-conditions entry's auto-fields win even + // when a later entry has different auto-fields (e.g. a second condition + // that matched a different `app_install_failed` event in the same + // session). Insertion order of matchedConditions mirrors rule.conditions + // order on the backend. The decoy below uses a different explicit `field` + // so the byField path doesn't cross-shadow — this asserts the auto-field + // shadowing semantics only. + const matchedConditions = { + esp_apps_detection_failure: { + eventId: "evt-office", + field: "failureType", + value: "esp_apps_detection_failure", + appName: "Microsoft Apps for Enterprise - 64Bit", + errorCode: "0x87d1041c", + }, + another_app_failure_decoy: { + eventId: "evt-other", + field: "isError", + value: "true", + appName: "Some Other App", + errorCode: "0x80070005", + }, + }; + + const out = interpolateRuleTemplate( + "App={{appName}} HR={{errorCode}}", + matchedConditions + ); + expect(out).toBe( + "App=Microsoft Apps for Enterprise - 64Bit HR=0x87d1041c" + ); + }); + + it("explicit field/value still wins over auto-fields from a later condition", () => { + // Defense-in-depth: if a downstream condition explicitly declared a + // dataField that happens to share a name with a whitelist auto-field, the + // explicit-field path (byField) still wins over the auto-field fallback, + // because byField is checked first. + const matchedConditions = { + explicit_appname: { + field: "appName", + value: "Explicit value", + }, + esp_apps_detection_failure: { + field: "failureType", + value: "esp_apps_detection_failure", + appName: "Auto-field value", + }, + }; + const out = interpolateRuleTemplate("{{appName}}", matchedConditions); + expect(out).toBe("Explicit value"); + }); + + it("ignores non-whitelist evidence keys (no leakage of internal fields)", () => { + // Evidence dicts may contain internal keys like `eventId`, `sequence`, + // `timestamp`, `field`, `value`, `eventType` — none of these may be resolved + // as template tokens, otherwise authors could accidentally leak Table + // Storage row keys into customer-visible rule explanations. + const matchedConditions = { + sig: { + eventId: "evt-secret", + sequence: 42, + timestamp: "2026-05-28T10:00:00Z", + eventType: "app_install_failed", + field: "failureType", + value: "esp_apps_detection_failure", + }, + }; + expect(interpolateRuleTemplate("{{eventId}}", matchedConditions)).toBe("{{eventId}}"); + expect(interpolateRuleTemplate("{{sequence}}", matchedConditions)).toBe("{{sequence}}"); + expect(interpolateRuleTemplate("{{timestamp}}", matchedConditions)).toBe("{{timestamp}}"); + expect(interpolateRuleTemplate("{{eventType}}", matchedConditions)).toBe("{{eventType}}"); + }); }); diff --git a/src/Web/autopilot-monitor-web/lib/interpolateRuleTemplate.ts b/src/Web/autopilot-monitor-web/lib/interpolateRuleTemplate.ts index 05fa6914..e274dcf9 100644 --- a/src/Web/autopilot-monitor-web/lib/interpolateRuleTemplate.ts +++ b/src/Web/autopilot-monitor-web/lib/interpolateRuleTemplate.ts @@ -5,9 +5,16 @@ * Resolution order for `{{token}}`: * 1. The first matchedConditions entry whose `field` equals `token` * (e.g. {{uefiCA2023Status}} -> entry where field === "uefiCA2023Status"). - * 2. A matchedConditions entry whose key (signal name) equals `token` + * 2. The first matchedConditions entry that carries a same-event auto-field + * named `token` (added by the backend's + * `RuleEngine.ConditionEvaluators.AddDataFieldsToEvidence` whitelist — + * currently `appId`, `appName`, `errorPatternId`, `errorCode`, `exitCode`, + * `status`). Using "first" pins these to the same source event as the + * rule's required condition, avoiding cross-event interpolation drift + * when a session has multiple `app_install_failed` events. + * 3. A matchedConditions entry whose key (signal name) equals `token` * (e.g. {{ca2023_not_updated}} -> that signal's value). - * 3. Unresolved tokens are left untouched so authors notice the typo. + * 4. Unresolved tokens are left untouched so authors notice the typo. * * Token chars: [a-zA-Z0-9_]. Whitespace inside the braces is tolerated. */ @@ -19,8 +26,15 @@ export function interpolateRuleTemplate( if (!matchedConditions || typeof matchedConditions !== "object") return text; const byField = new Map(); + const byAutoField = new Map(); const bySignal = new Map(); + // Whitelist of auto-injected event fields the backend rule engine adds to every + // matched-condition evidence dict via AddDataFieldsToEvidence. Mirror keeps the + // mapping explicit so a rogue evidence key (e.g. accidental `description`) can't + // shadow a template token from another resolution path. + const AUTO_FIELDS = ["appId", "appName", "errorPatternId", "errorCode", "exitCode", "status"]; + for (const [signal, evidence] of Object.entries(matchedConditions)) { if (!evidence || typeof evidence !== "object") { if (evidence != null) bySignal.set(signal, formatValue(evidence)); @@ -33,10 +47,19 @@ export function interpolateRuleTemplate( if ("value" in e) { bySignal.set(signal, formatValue(e.value)); } + // Auto-field fallback: first non-empty wins (insertion order of + // matchedConditions mirrors rule.conditions order, so the required + // condition's event is checked first). + for (const af of AUTO_FIELDS) { + if (byAutoField.has(af)) continue; + const v = e[af]; + if (v != null && v !== "") byAutoField.set(af, formatValue(v)); + } } return text.replace(/\{\{\s*([a-zA-Z0-9_]+)\s*\}\}/g, (raw, token: string) => { if (byField.has(token)) return byField.get(token)!; + if (byAutoField.has(token)) return byAutoField.get(token)!; if (bySignal.has(token)) return bySignal.get(token)!; return raw; });