fix(settings): don't let a stored bad filterNameRegex brick automations (#3934)

[Yeraze]

Summary

The auto-traceroute and remote-LocalStats node-filter POST endpoints hard-validated filterNameRegex with RE2 (compileUserRegex) on every save. RE2 rejects the lookaround/backreferences that the browser's native RegExp (client-side validation) accepts, so a pattern like ^(?!.*Mobile).*$ passes on the client, gets persisted, then fails server validation forever — the settings form re-POSTs the stored regex on every save, so the whole request 400s and the user can't even disable the filter or the automation. This is the same failure mode #3806 fixed for auto-ack; the guard was never ported to these two endpoints.

Fixes #3934.

Changes

• Port the Bug: Auto Acknowledge settings return 400 after saving other automation settings #3806 guard to POST /api/settings/traceroute-nodes and POST /api/settings/remote-localstats-nodes (src/server/server.ts): only run the RE2 check (and the ReDoS length/complexity caps) when the regex will actually be applied (the automation AND its regex sub-filter are enabled) or the incoming pattern differs from the stored one. Disabling the automation/filter, or re-saving an unchanged bad pattern, now succeeds; newly-supplied invalid patterns are still rejected when enabling/changing.
• Each endpoint now reads the current stored settings up-front (to compute "will be applied" from the effective filterRegexEnabled and to detect an unchanged re-save).
• Extract the shared guard to src/server/utils/filterNameRegex.ts (validateFilterNameRegexOnSave) so it can be unit-tested — the endpoints are defined inline in server.ts and aren't independently mountable.

Issues Resolved

Fixes #3934

Documentation Updates

No documentation changes needed.

Testing

• Full unit suite passes (8002 / 0, success: true)
• TypeScript compiles cleanly
• New filterNameRegex.test.ts (8 cases): rejects a new lookaround pattern when applied or changed; allows re-saving an unchanged stored bad pattern while disabled (the recovery path); allows clearing a bad pattern to a valid one; still rejects an unchanged bad pattern while it stays applied; ReDoS length/complexity caps
• Manual: persist a lookaround filterNameRegex (e.g. ^(?!.*Mobile).*$), then confirm toggling the filter off / disabling the automation now saves instead of returning 400

🤖 Generated with Claude Code

[github-actions]

Claude finished @Yeraze's task —— View job

---

PR Review: fix(settings): don't let a stored bad filterNameRegex brick automations (#3934)

Tasks:

• Gather context and understand the PR
• Read changed files
• Review code quality and logic
• Check test coverage
• Post review feedback

---

Overall this is a well-reasoned, conservative fix. The guard correctly ports the #3806 pattern without over-engineering it. A few notes below, ranging from a potential bug to minor style observations.

Potential bug — validatedRegex defaults to '.*' when filterNameRegex is absent from the request body

src/server/server.ts:3565 and src/server/server.ts:3761:

let validatedRegex = '.*';
if (filterNameRegex !== undefined && filterNameRegex !== null) {
  ...
  validatedRegex = regexResult.regex;
}
// then later:
filterNameRegex: validatedRegex,  // always written

If the client sends a request body that omits filterNameRegex entirely (e.g. a partial patch), the endpoint silently overwrites whatever was stored with '.*'. This was present before this PR, but the PR now reads currentTraceSettings (and currentRemoteSettings) — it would be trivial to preserve the stored value instead:

let validatedRegex = currentTraceSettings.filterNameRegex; // was: '.*'

This is low severity if the frontend always sends the field, but it's a latent footgun for any future caller that issues a partial save.

willBeApplied logic may be slightly over-strict at the traceroute endpoint

src/server/server.ts:3570-3572:

const regexWillBeApplied =
  enabled &&
  (filterRegexEnabled !== undefined ? filterRegexEnabled === true : currentTraceSettings.filterRegexEnabled);

The intent is: "regex will be applied after this save." But filterRegexEnabled from the body could be false (disabling the sub-filter), which correctly sets willBeApplied = false. The fallback to currentTraceSettings.filterRegexEnabled is also correct. This logic is sound. ✓

The equivalent block in the remote-LocalStats endpoint (src/server/server.ts:3766-3768) is identical in structure. ✓

Observation — filterNameRegex is not present in getTracerouteFilterSettingsAsync return type (worth verifying)

The guard reads currentTraceSettings.filterNameRegex as the storedRegex. If the settings getter can return undefined for that field (e.g. before a user has ever set it), the changed check in validateFilterNameRegexOnSave (incoming !== opts.storedRegex) would never match incoming and would always flag a change, meaning even saving an unchanged bad pattern while disabled would be rejected — defeating the recovery path. Worth confirming getTracerouteFilterSettingsAsync always returns a string (defaulting to '.*').

Minor — REDOS_PATTERN doesn't catch all catastrophic shapes

src/server/utils/filterNameRegex.ts:22:

const REDOS_PATTERN = /(\.\*){2,}|(\+.*\+)|(\*.*\*)|(\{[0-9]{3,}\})|(\{[0-9]+,\})/;

This heuristic is weaker than the DANGEROUS_SHAPE_RE in autoAckRegex.ts, which also blocks nested quantifiers like (a+)+. However, this is pre-existing in the autoAckRegex.ts guard philosophy and not a regression introduced here. If hardening is wanted later, compileUserRegex itself (RE2) would catch catastrophic backtracking anyway since it rejects many suspect patterns.

Test coverage ✓

filterNameRegex.test.ts covers all 8 meaningful branches:

• valid pattern applied ✓
• new RE2-incompatible pattern, applied ✓
• new RE2-incompatible pattern, not applied (still rejected because it changed) ✓
• unchanged bad pattern, not applied → allowed (the recovery path) ✓
• unchanged bad pattern, still applied → rejected ✓
• clearing bad pattern to valid → allowed ✓
• length cap ✓
• ReDoS heuristic ✓

The test at line 34 ('still rejects an unchanged stored bad pattern while the filter STAYS applied') is especially important for correctness and is present. Good coverage.

Minor — utility file location

src/server/utils/filterNameRegex.ts lives under src/server/utils/ but imports from ../../utils/safeRegex.js (the shared src/utils/ layer). The placement is fine — it's server-only logic. No issue.

Summary

The core logic is correct and the test suite is thorough. The one item worth addressing before merge is the validatedRegex default of '.*' when the field is absent — using the stored value instead is a one-line fix that prevents silent data loss on partial saves. Everything else is informational or pre-existing.