fix(theme): reset-to-factory follows the active base; theme names can't become paths

[nigelfenton]

#3184 item 4 — the last of the four. Two unrelated defects in the same area.

⚠ Stacked on #4573 (b16f6ba2), which touches the same file. The diff against main
therefore includes that PR's commit; review e206f0e1 alone, or merge #4573 first.

1. "Reset to default" restored DARK values while editing a light theme

ensureFactoryLoaded() was hardcoded to :/themes/default-dark.json and was a one-shot latch,
so the factory snapshot never reflected what the operator was actually editing.

Pressing Reset on Default Light restored the dark value for 96 of the 147 root tokens the two
bundled themes share:

token	Light's real value	what Reset gave you
color.background.0	#f5f5f8	#0f0f1a
color.accent	#0088b0	#00b4d8
color.accent.danger	#c02020	#ff4d4d
color.text.primary	…	… (and 92 more)

color.background.0 is the vivid one — Reset turned a near-white background near-black.

The baseline is now chosen from the active theme, and the snapshot re-loads when the base
changes instead of latching for the process (clearing first, so entries from the other base can't
leak through).

For a user theme there is no recorded parentage, so background luminance decides — a fork of Light
resets to Light's values. That is a heuristic and I've said so in the comment, but it is the closest
honest answer available and it is right for the case that matters.

2. A theme name typed by the operator went straight into a filename

saveCurrentThemeAs() and renameTheme() both build <themes-dir>/<name>.json with no
validation. saveCurrentThemeAs("../escape") succeeded before this change — verified in a test,
not theorised.

importThemeFromFile() already guards this exact case, and says why:

Sanitise so we can't path-traverse via a malicious name field — slashes get replaced with
underscores.

The rule simply was not applied at the two entry points where a human types the name.

These refuse rather than substitute — deliberately the opposite of what import does. Import's
name comes from a file's JSON with nobody watching, so substituting beats stranding a valid import.
Here the operator typed it, and silently saving their theme under a name they did not choose is
worse than telling them no.

Both separators are rejected on every platform: \ is not a separator on Unix, but a theme file is
portable and a name containing one becomes a traversal the moment it is opened on Windows.

Verification

Five cases in theme_manager_test, each verified red against the unfixed code, independently:

revert	failures
baseline selection only	exactly the 2 light-theme assertions — the dark ones keep passing
separator guards only	5, including ../escape being accepted and appearing in availableThemes()
nothing	0

A legitimate name with punctuation (Nigel's Theme (v2)) still saves, so the guard rejects
separators and not ordinary characters.

ctest -R "theme|s_meter|colour|color|style|applet" 5/5; full tree builds on Windows/MSVC.

That completes the four

1. ✅ audit_colours.py Windows fix — merged as fix(tools): make audit_colours.py run to completion on a legacy-code-page console #4475
2. 🔄 seed table generation — groundwork in ci(theme): pin the compiled theme seed against the bundled JSON #4570; generator blocked on two design questions
3. ✅ round-trip correctness — fix(theme): three silent round-trip losses in save / export / template resolve #4573
4. ✅ this

Refs #3184.

[aethersdr-agent]

Reviewed e206f0e1 alone (the #4573 commit is excluded from the notes below). Both defects are real and the framing is right: I confirmed :/themes/default-light.json is in resources/resources.qrc, that its primitives resolve to exactly the values the new assertions pin (color.background.0 → #f5f5f8, color.accent → #0088b0, vs #0f0f1a/#00b4d8 for dark), and that importThemeFromFile() already replaces both separators — so the "same rule, two missing entry points" story checks out, as does refusing rather than substituting where the operator typed the name. The re-snapshot-on-base-change keyed on m_factoryBaselinePath with a clear() first is the right shape.

Nothing here blocks on design. Three small things and a couple of polish notes.

Would like fixed before merge

• Missing space in the adjacent-string-literal concat in both new warnings — the log reads "refusing name witha path separator" (two inline comments, both with suggestions).
• const_cast<ThemeManager*>(this)->color(...) is unnecessary; color(const QString&) is already const.

Polish

• Header comment on m_factoryTokens still says "loaded once from :/themes/default-dark.json at construction" — the thing this PR just stopped being true.
• saveCurrentThemeAs() trims only for the emptiness check, so "My Theme " still becomes My Theme .json, unlike renameTheme() which trims throughout.

Non-blocking notes

• The luminance discriminator reads the live, edited color.background.0, so it can flip mid-session while the operator is editing — see inline; it's a heuristic either way, worth one line of comment rather than a code change.
• CodeGuard's three CG-PATH-001 hits on tests/theme_manager_test.cpp:960/966/975 are false positives: those are the "../escape" / "sub/dir" / "back\\slash" literals in the negative tests asserting the guard refuses them. No action needed.

Nice work on the red-against-unfixed-code verification table — the "revert baseline selection only → exactly the 2 light assertions fail, dark keeps passing" split is exactly the evidence that makes this easy to trust.

---

_{🤖 aethersdr-agent · cost: $5.3224 · model: claude-opus-5}

[aethersdr-agent]

ThemeManager::color(const QString&) const is already const (ThemeManager.h:112), so this const_cast isn't buying anything — and it makes the line read as if the lookup mutates state, which it doesn't.

Suggested change

	const QColor bg = const_cast<ThemeManager*>(this)
	->color(QStringLiteral("color.background.0"));
	const QColor bg = color(QStringLiteral("color.background.0"));

[aethersdr-agent]

Worth one line in the comment above: the discriminator reads the live, edited color.background.0, not the value the theme was loaded with. So an operator forking Default Dark and dragging the background past L=127 flips the baseline for all ~147 tokens mid-session — every subsequent Reset silently starts answering with Light's values, and m_factoryTokens gets cleared and re-snapshotted underneath them.

Not asking you to change the heuristic (there's no recorded parentage to consult, as you say). But reading the persisted file's background rather than the live token would make the baseline stable for the duration of an editing session, which is the property that actually matters here. If that's more than you want in this PR, just say so in the comment so the next reader knows it's deliberate.

[aethersdr-agent]

Adjacent string literals concatenate with no separator, so this logs refusing name witha path separator:.

Suggested change

	qCWarning(lcGui) << "ThemeManager::renameTheme: refusing name with"
	"a path separator:" << trimmed;
	qCWarning(lcGui) << "ThemeManager::renameTheme: refusing name with "
	"a path separator:" << trimmed;

[aethersdr-agent]

Same missing space as in renameTheme() — witha path separator.

Suggested change

	qCWarning(lcGui) << "ThemeManager::saveCurrentThemeAs: refusing name with"
	"a path separator:" << newThemeName;
	qCWarning(lcGui) << "ThemeManager::saveCurrentThemeAs: refusing name with "
	"a path separator:" << newThemeName;

[aethersdr-agent]

Polish, pre-existing but adjacent to what you're touching: the emptiness check on the line above trims, and everything after this uses newThemeName raw — so "My Theme " writes My Theme .json and registers a trailing-space key in m_themePaths, while renameTheme() trims throughout. A const QString name = newThemeName.trimmed(); at the top of the function, used for the guard and the path, would make the two entry points agree. Fine to leave for a follow-up.

[aethersdr-agent]

This comment is now the thing the PR fixed — it's no longer loaded once, and no longer from default-dark.json specifically.

Suggested change

	// Factory-default snapshot, loaded once from `:/themes/default-dark.json`
	// at construction. Drives the gradient editor's "Reset to default"
	// button. Lazy-initialised so a totally missing resource bundle
	// doesn't take the whole singleton down.
	// Factory-default snapshot of the bundled theme the ACTIVE theme
	// descends from — see factoryBaselinePath(). Drives the gradient
	// editor's "Reset to default" button. Lazily (re)loaded so a totally
	// missing resource bundle doesn't take the whole singleton down.

[aethersdr-agent]

Cisco CodeGuard — static analysis of this PR (3 finding(s))

• [MEDIUM] CG-PATH-001 — Potential path traversal in tests/theme_manager_test.cpp /tmp/aetherclaude/pr-4575/tests/theme_manager_test.cpp:960
• [MEDIUM] CG-PATH-001 — Potential path traversal in tests/theme_manager_test.cpp /tmp/aetherclaude/pr-4575/tests/theme_manager_test.cpp:966
• [MEDIUM] CG-PATH-001 — Potential path traversal in tests/theme_manager_test.cpp /tmp/aetherclaude/pr-4575/tests/theme_manager_test.cpp:975

_{Automated static scan by Cisco DefenseClaw CodeGuard on the changed files. Advisory — some may be false positives; the review above verifies them.}

---

_{🤖 aethersdr-agent · cost: $5.4439 · model: claude-opus-5}