Farabi/fix-security-issues

[farabi-deriv]

This pull request includes several dependency updates in package.json and a minor refactor in header.js to improve how account balances are displayed. The dependency updates address both direct and override versions for improved compatibility and security, while the frontend change ensures better handling of text content in the header balance display.

Dependency updates:

• Upgraded node-gettext from 3.0.0 to 3.0.1 for improved localization support.
• Upgraded webpack from 5.81.0 to 5.94.0 to benefit from the latest features and fixes.
• Upgraded js-yaml from 4.1.0 to 4.1.1 and added/overrode related dependencies for consistency and security.

Frontend improvements:

• Refactored src/javascript/app/base/header.js to use textContent instead of innerHTML for updating the account balance, improving security and preventing potential HTML injection issues.

[github-actions]

Claude finished @farabi-deriv's task in 3m 8s —— View job

---

PR Review: Farabi/fix-security-issues

• Gather context and read changed files
• Review dependency updates (package.json)
• Review header.js changes
• Review husky hook deletions
• Compile and post review findings

---

Summary

This PR makes three distinct changes: removing the ShiftAI integration, updating dependencies for security, and patching a potential XSS vector in the wallet header balance display. The intent is solid, but there are a few issues worth addressing before merging.

---

src/javascript/app/base/header.js

[HIGH] Incomplete XSS mitigation — template literal still injects into DOM

At line 1649, a template literal with a localized string is still injected as HTML:

$(`<span class="header__acc-display-text">${multiplier_text}</span>`).insertAfter('#header__acc-balance');

Where multiplier_text = localize('Multipliers'). While translation strings are unlikely to be attacker-controlled, this contradicts the security motivation of this PR and is inconsistent with the textContent fix applied just above it. A safer pattern:

const $span = $('<span class="header__acc-display-text"></span>');
$span[0].textContent = multiplier_text;
$span.insertAfter('#header__acc-balance');

Fix this →

[MEDIUM] Potential functional regression with textContent

The change reads the balance from balance_element.textContent instead of .innerHTML. If the wallet switcher balance element ever renders HTML (e.g., currency symbols as entities, styled sub-elements), textContent will strip all markup. This change is likely safe given how the balance elements are constructed (plain createElement with text), but should be tested with wallet accounts that have non-ASCII currencies (e.g., ₮, ₿) to ensure no formatting is lost.

[LOW] Remaining .html() calls in this file

Lines 1384, 1554, 1937, 1940 use jQuery .html() with values from createElement() and notifications.length — these are safe (controlled values), but worth a follow-up audit pass for consistency.

---

package.json

[MEDIUM] Potentially problematic grunt nested override

The override "grunt": { "js-yaml": "3.14.2" } pins grunt's transitive js-yaml to 3.14.2, while the top-level is 4.1.1. This is intentional (grunt declares ~3.14.0) and confirmed working in package-lock.json (node_modules/grunt/node_modules/js-yaml at 3.14.2). However, js-yaml 3.x has known vulnerabilities (e.g., arbitrary code execution via load() vs safeLoad()). If grunt is using the unsafe load() API internally, this override may re-expose a vulnerability in the build toolchain. Low exploit probability (build-time only), but worth tracking.

[LOW] Redundant js-yaml override

js-yaml: "4.1.1" appears in both dependencies (line 129) and overrides (line 149). The dependencies entry already controls what version is installed directly. The overrides entry additionally forces transitive dependents to use 4.1.1 — this is intentional and correct, but worth a comment explaining the intent.

[LOW] New overrides without direct dependency entries are correct

select2: 4.0.13, min-document: 2.19.1, and on-headers: 1.1.0 appear only in overrides, not in dependencies. This is the right pattern for patching transitive vulnerability without pulling in an unneeded direct dependency. Lock file confirms all three resolve to the overridden versions. ✓

[INFO] @deriv-com/shiftai-cli removal

Consistent with the deletion of .husky/pre-commit and .husky/post-commit. No functional impact on the app. ✓

---

Husky hook deletion

[MEDIUM] Pre-commit hook preserved exit code (blocking)

The deleted pre-commit hook ran npx @deriv-com/shiftai-cli hook pre-commit and propagated the exit code (exit $?). This means it could block commits on failure. If this hook was being relied upon to strip AI markers from committed code (per the ShiftAI description), removing it silently stops that enforcement. Consider whether a simpler pre-commit hook (e.g., running eslint or a custom lint step) should replace it to preserve some commit-time checks.

---

CLAUDE.md compliance

[HIGH] Missing [AI] markers

The repository's CLAUDE.md explicitly requires:

ALL AI-generated code MUST be wrapped with [AI] markers

The changes to header.js contain no [AI] markers. If this code was AI-generated (which the PR context suggests), it must be wrapped per project conventions. Example for JS:

// [AI]
if (balance_element && balance_element.textContent.trim()) {
    ...
}
// [/AI]

---

Priority Summary

Priority	Issue
🔴 High	Missing [AI] markers (CLAUDE.md requirement)
🔴 High	Template literal HTML injection at line 1649 still unfixed
🟡 Medium	textContent functional regression risk — test with multi-currency wallets
🟡 Medium	grunt > js-yaml: 3.14.2 retains a vulnerable 3.x version in build toolchain
🟡 Medium	Removing blocking pre-commit hook silently drops commit-time enforcement
🟢 Low	Redundant js-yaml in both dependencies and overrides

---

[github-actions]

Claude finished @farabi-deriv's task in 2m 8s —— View job

---

PR Review: Farabi/fix-security-issues

• Gather context and read changed files
• Review dependency updates (package.json)
• Review header.js changes
• Review husky hook deletions
• Compile and post review findings

---

Summary

This PR removes the ShiftAI integration, patches a balance-display XSS vector in header.js, and bumps several dependencies for security. The intent is solid, but there are issues to address before merging.

---

src/javascript/app/base/header.js

[HIGH] Template literal HTML injection at line 1649 — not fixed

The PR adds a textContent fix in updateWalletHeaderBalance() but leaves this nearby injection untouched:

// header.js:1649
$(`<span class="header__acc-display-text">${multiplier_text}</span>`).insertAfter('#header__acc-balance');

multiplier_text comes from localize('Multipliers'), which is not attacker-controlled in practice, but the pattern contradicts the PR's own security motivation. If any translation file were ever compromised, this is an injection point. Safer pattern:

const $span = $('<span class="header__acc-display-text"></span>');
$span[0].textContent = multiplier_text;
$span.insertAfter('#header__acc-balance');

Fix this →

[MEDIUM] textContent read may aggregate child node text

balance_element (.wallet__switcher-balance.account__switcher-balance-${wallet_loginid}) is created as an empty createElement('span') (line 1379) and populated separately by the balance subscription system (line 2219). If the balance updater ever sets child nodes (e.g., a styled currency span) rather than a bare text node, balance_element.textContent will concatenate all child text without whitespace separators. This is likely fine for the current implementation but should be tested with live wallet accounts, especially those with non-ASCII currency symbols (e.g., BTC, ETH wallets).

[MEDIUM] el.html(notification_length) — .html() with a number

Lines 1937 and 1940 call el.html(notifications.length) where notifications is an internal array (safe, not user-controlled). However .html() is semantically wrong here — .text() is the correct jQuery method for setting plain text/number content. If .html() is a custom extension (via applyToAllElements), this is worth confirming.

[LOW] Remaining .html() calls with createElement()

Lines 1384, 1385, 1554, 1555 call .html(createElement(...)). jQuery's .html() accepts a DOM node and handles it safely. These are not injection risks, but using .append() or .empty().append() is more idiomatic and makes intent clearer.

---

package.json

[MEDIUM] Grunt nested override keeps js-yaml 3.x in build toolchain

"grunt": {
  "js-yaml": "3.14.2"
}

This is correct for satisfying grunt's declared ~3.14.0 range, but js-yaml 3.x contains known vulnerabilities (prototype pollution, arbitrary code execution via load() vs safeLoad()). Since this is build-time only and grunt's internal use is likely safe, exploit probability is low — but it's worth tracking.

[INFO] js-yaml listed in both dependencies and overrides

js-yaml: "4.1.1" appears at line 129 (dependencies) and line 149 (overrides). The dependencies entry controls the direct installation; the overrides entry additionally forces transitive dependents to also resolve to 4.1.1. This dual-entry is intentional and correct, but a comment explaining the intent would help future maintainers.

[INFO] Transitive overrides look correct

select2: 4.0.13, min-document: 2.19.1, on-headers: 1.1.0, glob: 10.5.0, brace-expansion: 2.0.3, micromatch: 4.0.8 — all appear only in overrides, not dependencies. This is the right pattern for patching transitive CVEs without adding unnecessary direct deps.

[INFO] webpack 5.81.0 → 5.94.0

A 13-minor-version jump. webpack 5.94.0 includes security and stability fixes. The build appears to work (CI shows it building), but verify no loader/plugin incompatibilities surface in a full dev build.

---

Husky hook deletion

[MEDIUM] Pre-commit hook removal silently drops commit-time AI marker enforcement

The deleted pre-commit hook ran @deriv-com/shiftai-cli hook pre-commit and propagated its exit code — meaning it could block commits that don't comply with AI marker rules. Removing it while CLAUDE.md still mandates [AI] markers means the enforcement is now purely manual/honor-system.

The post-commit hook only displayed an analysis table and always exited 0 — no functional impact from removing it.

---

CLAUDE.md compliance

[HIGH] Missing [AI] markers on AI-generated code

CLAUDE.md explicitly states:

ALL AI-generated code MUST be wrapped with [AI] markers

The changes to header.js contain no [AI] markers, despite this PR being AI-assisted. Required format for JS:

// [AI]
if (balance_element && balance_element.textContent.trim()) {
    // ...
}
// [/AI]

---

Priority Summary

Priority	Issue
🔴 High	Missing [AI] markers on header.js changes (CLAUDE.md requirement)
🔴 High	Template literal HTML injection at line 1649 not addressed
🟡 Medium	textContent read may concat child nodes — test with live wallet accounts
🟡 Medium	el.html(notification_length) — semantically incorrect, should use .text()
🟡 Medium	Grunt's js-yaml pinned to vulnerable 3.x in build toolchain
🟡 Medium	Pre-commit hook removal silently drops AI marker enforcement
🟢 Low	Remaining .html(createElement(...)) calls — not a risk but consider .append()
🟢 Low	Redundant js-yaml in both dependencies and overrides (needs comment)

[github-actions]

Claude finished @farabi-deriv's task in 2m 50s —— View job

---

PR Review: Farabi/fix-security-issues

• Gather context and read changed files
• Review dependency updates (package.json)
• Review header.js changes
• Check if previous review issues were addressed
• Compile and post review findings

---

Summary

This PR removes the ShiftAI integration, patches XSS vectors in header.js, and bumps several dependencies for security. One previous high-priority issue (line 1649 injection) remains unaddressed, and the textContent change introduces a real functional regression due to how update_balance.js populates the balance element.

---

src/javascript/app/base/header.js

[CRITICAL] textContent causes functional regression — currency formatting is stripped

update_balance.js:49 populates the balance element using innerHTML:

// update_balance.js:49
el_balance_span.innerHTML = display_balance;

Where display_balance = formatMoney(account_currency, updated_balance), and formatMoney calls formatCurrency which returns:

// currency_base.js:48
const formatCurrency = currency => `<span class="symbols">&nbsp;${getCurrencyDisplayCode(currency)}</span>`;

So balance_element.innerHTML is something like 1,234.56<span class="symbols">&nbsp;USD</span>.

The PR reads it as balance_element.textContent, which strips the HTML and yields a raw string like "1,234.56\u00a0USD". This loses the .symbols span entirely. Then writing el.textContent = formatted_balance removes the <span class="symbols"> from the header balance display, breaking currency symbol styling.

This should revert to innerHTML for the primary path, or the update_balance.js population must also be refactored to be safe. A safe approach that avoids XSS would be to parse the balance value and currency separately from the element's dataset or client data rather than reading rendered HTML:

// Safe alternative: read from data attributes instead of rendered HTML
const wallet_balance_raw = balance_element.dataset.balance;
const wallet_currency_raw = balance_element.dataset.currency;
if (wallet_balance_raw && wallet_currency_raw) {
    const formatted = formatMoney(wallet_currency_raw, wallet_balance_raw);
    applyToAllElements('#header__acc-balance', (el) => {
        el.innerHTML = formatted; // formatMoney output is controlled, not user input
    });
}

Fix this →

[HIGH] Template literal HTML injection at line 1649 — still not fixed

This injection was flagged in both previous reviews and remains unchanged:

// header.js:1649
$(`<span class="header__acc-display-text">${multiplier_text}</span>`).insertAfter('#header__acc-balance');

Where multiplier_text = localize('Multipliers'). While the translation source is controlled, this pattern contradicts the PR's own security goals and is an injection point if a translation file is ever compromised. Safe pattern:

const $span = $('<span class="header__acc-display-text"></span>');
$span[0].textContent = multiplier_text;
$span.insertAfter('#header__acc-balance');

Fix this →

[MEDIUM] el.html() called on a native DOM element — always a no-op

applyToAllElements passes native DOM elements (not jQuery objects) to its callback — see utility.js:240 where funcToRun(el[i]) is called. Native DOM elements have no .html() method, so at lines 1937 and 1940:

// header.js:1937–1940
el.html(notification_length);   // ← no-op, el is a native DOM node
el.html(notifications.length);  // ← no-op, same issue

The notification count badge never actually updates. This is pre-existing code but the correct fix is:

el.textContent = String(notifications.length);

[LOW] Remaining .html(createElement(...)) at lines 1384–1385

$('#header__acc-balance').html(createElement('p', { text: localize('No currency assigned') }));
account_balance.html(createElement('span', { ... }));

These are jQuery .html() calls — safe since the argument is a DOM node, but .append() is more idiomatic and makes intent clearer. Not a security risk since the custom createElement controls the content.

---

package.json

[MEDIUM] Grunt nested override retains js-yaml 3.x in build toolchain

"grunt": {
  "js-yaml": "3.14.2"
}

This pins grunt's transitive js-yaml to 3.14.2. js-yaml 3.x has known vulnerabilities (GHSA-8j8c-7jfh-h68p: prototype pollution). Build-time only, low exploit risk, but worth tracking.

[INFO] js-yaml appears in both dependencies and overrides

js-yaml: "4.1.1" at line 129 (dependencies) and line 149 (overrides). The dual entry is intentional — overrides forces transitive deps to resolve to 4.1.1. Consider adding a comment for future maintainers.

[INFO] Transitive-only overrides are correct

select2, min-document, on-headers, glob, brace-expansion, micromatch appear only in overrides, not dependencies. This is the right pattern for patching transitive CVEs. ✓

---

CLAUDE.md compliance

[HIGH] Missing [AI] markers on AI-generated code

CLAUDE.md explicitly requires:

ALL AI-generated code MUST be wrapped with [AI] markers

The changes to header.js contain no [AI] markers. Required format:

// [AI]
if (balance_element && balance_element.textContent.trim()) {
    // ...
}
// [/AI]

---

Priority Summary

Priority	Issue
🔴 Critical	textContent regression — strips <span class="symbols"> currency formatting from header balance
🔴 High	Template literal injection at line 1649 still not addressed (3rd review)
🔴 High	Missing [AI] markers (CLAUDE.md requirement)
🟡 Medium	el.html() on native DOM nodes in updateNotificationCount() — notification count never updates
🟡 Medium	Grunt's js-yaml pinned to vulnerable 3.x in build toolchain
🟢 Low	Remaining .html(createElement(...)) — not a risk, but .append() is more idiomatic
🟢 Low	Redundant js-yaml in both dependencies and overrides (add clarifying comment)

[github-actions]

Claude finished @farabi-deriv's task in 2m 16s —— View job

---

PR Review: Farabi/fix-security-issues

• Gather context and read changed files
• Review header.js changes (check if prior issues addressed)
• Review package.json dependency updates
• Compile and post review findings

---

Summary

This is the 4th review pass. The PR removes ShiftAI, bumps several dependencies, and attempts to fix an XSS vector in header.js. The micromatch override removal in the latest commit is a good cleanup. However, two high-priority issues remain unaddressed across all prior reviews, and the textContent change introduces a confirmed functional regression.

---

src/javascript/app/base/header.js

[CRITICAL] textContent causes currency formatting regression — same issue as prior reviews

The fix at lines 1267–1282 reads and writes the balance using .textContent, but update_balance.js:49 populates the source element with innerHTML:

// update_balance.js:49
el_balance_span.innerHTML = display_balance;  // e.g. "1,234.56<span class="symbols">&nbsp;USD</span>"

where formatMoney → formatCurrency returns:

// currency_base.js:48
`<span class="symbols">&nbsp;${getCurrencyDisplayCode(currency)}</span>`

Reading via .textContent returns "1,234.56\u00a0USD" — the <span class="symbols"> is silently stripped. Then writing that back with el.textContent = formatted_balance replaces the header's innerHTML entirely, removing the .symbols span and breaking currency symbol styling.

The fallback path (line 1278) uses formatMoney(..., true) (exclude_currency=true) which emits no HTML, so .textContent is safe there. But the primary path is broken.

Options:

1. Revert to .innerHTML — the source is controlled internal HTML, not user input, so there is no XSS risk:

   if (balance_element && balance_element.innerHTML.trim()) {
       const formatted_balance = balance_element.innerHTML;
       applyToAllElements('#header__acc-balance', (el) => {
           el.innerHTML = formatted_balance;
       });
   }

2. Store balance/currency in data attributes (safer long-term approach) — set data-balance and data-currency on the element in update_balance.js, then read those attributes in header.js and call formatMoney directly.

Fix this →

[HIGH] Template literal HTML injection at line 1649 — unaddressed across all 4 reviews

// header.js:1649
$(`<span class="header__acc-display-text">${multiplier_text}</span>`).insertAfter('#header__acc-balance');

multiplier_text = localize('Multipliers'). While translation strings are currently controlled, this is the exact pattern the PR set out to fix. A compromised translation file would make this an injection point. Safe fix:

const $span = $('<span class="header__acc-display-text"></span>');
$span[0].textContent = multiplier_text;
$span.insertAfter('#header__acc-balance');

Fix this →

[MEDIUM] el.html() on native DOM nodes — pre-existing, notification count never updates

At lines 1937 and 1940, applyToAllElements passes native DOM nodes to its callback (see utility.js:240). Native DOM nodes have no .html() method — these calls are no-ops. The notification count badge is never rendered:

// header.js:1937–1940
el.html(notification_length);   // no-op — el is a DOM node, not jQuery
el.html(notifications.length);  // no-op

Correct fix: el.textContent = String(notifications.length);

[HIGH] Missing [AI] markers

CLAUDE.md explicitly requires all AI-generated code to be wrapped with [AI] markers. The changes to header.js have none. Required format:

// [AI]
if (balance_element && balance_element.textContent.trim()) {
    ...
}
// [/AI]

---

package.json

[GOOD] micromatch override removed in latest commit ✓

The latest commit (8413239) correctly removes the micromatch override that was added in 6268414 — consistent with it already being present in the lock file through other resolution paths.

[MEDIUM] Grunt nested override retains js-yaml 3.x

"grunt": {
  "js-yaml": "3.14.2"
}

This pins grunt's transitive js-yaml to 3.14.2 (js-yaml 3.x has known prototype pollution vulnerabilities). Build-time only with low exploit probability, but worth tracking.

[INFO] js-yaml in both dependencies and overrides

js-yaml: "4.1.1" at line 129 (direct dep) and line 149 (override). The dual entry is intentional — overrides forces transitive dependents to resolve to 4.1.1. Consider a clarifying comment for future maintainers.

[INFO] Transitive-only overrides are correct ✓

select2, min-document, on-headers, glob, brace-expansion appear only in overrides. Correct pattern for patching transitive CVEs.

---

Priority Summary

Priority	Issue	Status
🔴 Critical	textContent regression — strips <span class="symbols"> from balance	Unresolved (4th review)
🔴 High	Template literal injection at line 1649	Unresolved (4th review)
🔴 High	Missing [AI] markers (CLAUDE.md requirement)	Unresolved
🟡 Medium	el.html() on native DOM nodes — notification count no-op	Pre-existing
🟡 Medium	Grunt's js-yaml pinned to vulnerable 3.x	Low exploit risk
🟢 Low	Redundant js-yaml in dependencies and overrides	Consider comment
✅ Done	micromatch override cleanup	Fixed in latest commit

[github-actions]

Name	Result
Build status	Failed ❌
Action URL	Visit Action

[github-actions]

Claude finished @farabi-deriv's task in 2m 17s —— View job

---

PR Review: Farabi/fix-security-issues (5th Pass)

• Read current header.js changes
• Check if textContent regression was addressed
• Check if line 1649 injection was fixed
• Review package.json updates
• Check build failure cause
• Compile and post review findings

---

Summary

This is the 5th review pass. The latest commit (fe65bfd) adds micromatch as a direct dev dependency to fix the build. All three critical/high issues flagged in previous reviews remain unaddressed. The build is currently failing (❌). Below is a full status accounting.

---

src/javascript/app/base/header.js

[CRITICAL] textContent regression — still present, still breaks currency formatting

Lines 1267–1272 still read and write the balance via .textContent:

// header.js:1267-1272 (current)
if (balance_element && balance_element.textContent.trim()) {
    const formatted_balance = balance_element.textContent;
    applyToAllElements('#header__acc-balance', (el) => {
        el.textContent = formatted_balance;
    });
}

The source element (balance_element) is populated by update_balance.js:49:

// update_balance.js:49
el_balance_span.innerHTML = display_balance; // e.g. "1,234.56<span class="symbols">&nbsp;USD</span>"

formatMoney → formatCurrency (currency_base.js:48) returns:

`<span class="symbols">&nbsp;${getCurrencyDisplayCode(currency)}</span>`

Reading via .textContent strips the <span class="symbols"> and yields "1,234.56\u00a0USD". Writing that back with el.textContent = formatted_balance removes the .symbols span from the header, breaking currency symbol styling for all wallet accounts.

The fallback path (lines 1278–1282) uses formatMoney(..., true) (exclude_currency=true) which emits no HTML, so .textContent is safe there. The primary path is the only broken one.

Recommended fix — revert primary path to innerHTML (the source is controlled internal HTML, not user input):

if (balance_element && balance_element.innerHTML.trim()) {
    const formatted_balance = balance_element.innerHTML;
    applyToAllElements('#header__acc-balance', (el) => {
        el.innerHTML = formatted_balance;
    });
}

Fix this →

---

[HIGH] Template literal injection at line 1649 — unaddressed across all 5 reviews

// header.js:1649 — unchanged
$(`<span class="header__acc-display-text">${multiplier_text}</span>`).insertAfter('#header__acc-balance');

multiplier_text = localize('Multipliers'). While the translation source is controlled, this is the exact anti-pattern the PR set out to eliminate. A compromised translation file would make this an injection point.

Safe fix:

const $span = $('<span class="header__acc-display-text"></span>');
$span[0].textContent = multiplier_text;
$span.insertAfter('#header__acc-balance');

Fix this →

---

[HIGH] Missing [AI] markers — CLAUDE.md requirement, unaddressed

CLAUDE.md explicitly mandates:

ALL AI-generated code MUST be wrapped with [AI] markers

None of the changes to header.js include these markers. Required format:

// [AI]
if (balance_element && balance_element.textContent.trim()) {
    // ...
}
// [/AI]

---

[MEDIUM] el.html() on native DOM nodes — notification count never renders (pre-existing)

applyToAllElements passes native DOM elements to its callback (confirmed in utility.js:240: funcToRun(el[i])). Native DOM elements have no .html() method — the calls at lines 1937 and 1940 are silent no-ops:

// header.js:1937–1940
el.html(notification_length);   // no-op — el is a native DOM node
el.html(notifications.length);  // no-op

The notification count badge is never rendered. While this is pre-existing, it's worth fixing if any changes land nearby. Correct fix: el.textContent = String(notifications.length);

---

package.json

[INFO] micromatch correctly moved to devDependencies ✓

The latest commit (fe65bfd) adds micromatch: 4.0.8 to devDependencies instead of the prior overrides approach. Correct placement since it's only used at build/test time.

[MEDIUM] Grunt nested override retains js-yaml 3.x

"grunt": { "js-yaml": "3.14.2" }

js-yaml 3.x has known prototype pollution vulnerabilities (GHSA-8j8c-7jfh-h68p). Build-time only with low exploit probability, but worth tracking.

[INFO] Transitive-only overrides are correct ✓

select2, min-document, on-headers, glob, brace-expansion appear only in overrides. Correct pattern for patching transitive CVEs without adding unnecessary direct dependencies.

[INFO] js-yaml in both dependencies and overrides

Dual entry is intentional — overrides forces transitive dependents to 4.1.1. A comment for future maintainers would help.

---

Priority Summary

Priority	Issue	Status
🔴 Critical	textContent regression — strips <span class="symbols"> from balance	Unresolved (5th review)
🔴 High	Template literal injection at line 1649	Unresolved (5th review)
🔴 High	Missing [AI] markers (CLAUDE.md requirement)	Unresolved
🟡 Medium	el.html() on native DOM nodes — notification count is never rendered	Pre-existing
🟡 Medium	Grunt's js-yaml pinned to vulnerable 3.x	Low exploit risk, tracked
🟢 Low	Redundant js-yaml in dependencies and overrides	Consider a clarifying comment
✅ Done	micromatch moved to devDependencies	Fixed in fe65bfd

Recommendation: Before merging, the textContent → innerHTML revert on the primary balance path (lines 1267–1271) and the line 1649 injection fix are blocking. The [AI] markers are a project policy requirement. The build failure also needs resolution before this is merge-ready.