Skip to content

fix: add null checks for tactic/technique toLowerCase calls#201

Merged
Polliog merged 2 commits intologtide-dev:mainfrom
impsislegobatman:fix/null-check-tolowercase
Apr 16, 2026
Merged

fix: add null checks for tactic/technique toLowerCase calls#201
Polliog merged 2 commits intologtide-dev:mainfrom
impsislegobatman:fix/null-check-tolowercase

Conversation

@impsislegobatman
Copy link
Copy Markdown
Contributor

Fixes #200

Added optional chaining (?.) before .toLowerCase() calls on tactic and technique fields that the backend sometimes returns as null.

Backend sometimes returns null for tactic/technique fields, causing
'Cannot read properties of null' errors. Added optional chaining.

Fixes logtide-dev#200
@Polliog
Copy link
Copy Markdown
Collaborator

Polliog commented Apr 16, 2026

Hi @impsislegobatman, thanks for taking a stab at this!

A couple of notes before we can merge:

  1. The crash reported in [BUG] Cannot read properties of null (reading 'toLowerCase') #200 originates in packages/frontend/src/lib/components/siem/dashboard/MitreHeatmap.svelte:40, where tactic.toLowerCase() is called directly on a cell's tactic field before getTacticName() is ever invoked. So adding ?. inside the shared getTacticName / getTacticDescription / etc. won't actually prevent the reported crash: it would still throw one line earlier in abbreviateTactic.

  2. The change in packages/backend/src/modules/sigma/routes.ts guards params.tactic?.toLowerCase(), but params.tactic is a required route path parameter, so if the route matches it's always a string and the optional chaining there is unreachable.

  3. We've confirmed with the reporter (see [BUG] Cannot read properties of null (reading 'toLowerCase') #200) that the null values come from actual detection events stored in the DB after importing the "Auth and Security Pack" community rules, so the real fix needs to either drop those cells in the heatmap or backfill a sensible default.

We've already pushed a fix in release/0.9.2 that filters out cells with null tactic/technique in MitreHeatmap and hardens DetectionEventsList.getLogLevelClass. That said, your defensive ?. additions in shared/utils/mitre.ts are reasonable hygiene for other callers. If you want, we can keep that part and drop the other two changes. Let us know!

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

The route parameter is required, so the optional chaining is
unreachable. Keeping only the defensive changes in shared/utils/mitre.ts
as suggested in review.
@Polliog Polliog merged commit 784f9cf into logtide-dev:main Apr 16, 2026
6 checks passed
@Polliog
Copy link
Copy Markdown
Collaborator

Polliog commented Apr 16, 2026

Thanks for following up on the feedback so quickly! The trim to keep just the defensive changes in shared/utils/mitre.ts is exactly what we discussed clean second commit too.

Quick note on context: the actual crash from #200 (Cannot read properties of null from the MITRE heatmap) was fixed in #203 by filtering out null tactic/technique cells in MitreHeatmap.svelte, which just landed on main. So #200 is already resolved at the source. Merging this PR as additional hardening your ?. additions still help any future caller of these helpers that might receive a null/undefined value, so it's good defensive code to have.
Appreciate the contribution! 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Cannot read properties of null (reading 'toLowerCase')

2 participants