fix: harden sanitization, CLI parsing, retries, and web robustness#293
Merged
Conversation
…p fetch Extend the parse-boundary sanitizer to strip ANSI/control characters from every attacker-influenceable free-text field (device/channel status, type, firmware, sessionLabel, notes, and the archive equivalents), not just labels. This closes a terminal control-character injection vector (CWE-150) in CLI output where those fields were printed raw. Also fetch device-group documents in parallel instead of sequentially; Promise.all preserves order and the existing 404 handling while avoiding N sequential round trips. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ed65448a-d217-4a10-996d-a873a14f6e32
computeRetryDelay documented Retry-After as a floor, but full jitter was applied over max(exponential, retryAfter), so the delay could fall below the server-requested wait. Jitter only the exponential component and clamp the result to at least retryAfterMs (still capped at maxDelay), so a 429/503 carrying Retry-After is never retried early. Adds regression tests covering the floor and its cap. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ed65448a-d217-4a10-996d-a873a14f6e32
Commands resolved their positional (serial/meat/value) with
args.find(a => !a.startsWith("--")), which mistakes a value-taking
flag's value for the positional when the flag comes first, e.g.
`thermoworks eta --target 203 ABC123` queried device "203". Add a shared
firstPositional(args, valueFlags) helper that skips value flags and
their values, and use it in eta, temp, stall, alarm-suggest, and
convert. Adds unit tests for the helper.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ed65448a-d217-4a10-996d-a873a14f6e32
…ror state - export: add carriage return to the CSV formula-injection prefix set, matching the CLI escaper. - cook-annotations: wrap the localStorage write in try/catch so a quota or private-mode error can't break the report UI, matching the reader. - offline-mutations: wrap each queued replay in try/catch so one failing mutation no longer blocks replay of the rest (adds a regression test). - SharedArchiveView: clear stale archive state on a failed or absent fetch so a previous archive can't linger on an invalid link. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ed65448a-d217-4a10-996d-a873a14f6e32
Match the pnpm version pinned in package.json (11.14.0). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ed65448a-d217-4a10-996d-a873a14f6e32
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A focused hardening pass across the SDK, CLI, and web packages. It closes a terminal control-character injection vector, fixes a CLI argument-parsing bug that could target the wrong device, makes retry backoff respect the server's
Retry-After, and tightens several web error-handling paths. There are no changes to happy-path rendering or public API signatures.Changes
SDK
sanitizeLabelpreviously covered only device/channel labels; it now also strips ANSI/control characters fromstatus,type,firmware,sessionLabel,notes, and the archive equivalents, which several CLI commands printed raw (CWE-150).Retry-Afteras a hard floor incomputeRetryDelay: jitter only the exponential component and clamp to at least the server-requested wait (still capped atmaxDelay), so a 429/503 is never retried early.CLI
firstPositional(args, valueFlags)helper and use it ineta,temp,stall,alarm-suggest, andconvert. The oldargs.find(a => !a.startsWith("--"))treated a value flag's value as the positional when the flag came first, soeta --target 203 ABC123queried device "203".Web
\rto the CSV formula-injection prefix set (parity with the CLI escaper).cook-annotationslocalStorage write in try/catch (matches the reader).Docs
package.json(11.14.0).Testing
All checks green locally:
firstPositional, SDKRetry-Afterfloor, SDK parse-boundary sanitization, web offline-replay resilience.Playwright e2e/a11y/visual weren't run locally: every change is non-visual (escaping, try/catch, replay logic, error-path state, parse sanitization), so happy-path rendering and the committed visual baselines are unaffected.
Type of change
Security
eval/innerHTML/dangerouslySetInnerHTML/execNotes for reviewers
Surfaced during review and deliberately left for follow-ups (not in this PR): web account tokens in
localStorage(backendless-SPA tradeoff),UserManagementinvite removal targeting the users collection (needs the correct cloud endpoint), dialog focus-trap and list-semantics a11y, and MCP/watchfunction-size refactors.