fix: round-K audit follow-ups (input hardening + config robustness)#20
Merged
Conversation
- isValidRemoteName(): reject custom remote names that ssh would parse as flags (`-oProxyCommand=…`), have whitespace, or are empty. Wired into the interactive add-remote prompt with a user-facing rejection message. All three ssh spawn sites now also pass `--` before the remote, so even if a `-`-prefixed name slipped past the validator (e.g. via a hand-edited config.json), ssh wouldn't treat it as flags. - parseSSHConfig() now follows `Include` directives recursively. The loader is injected (testable). Real-world loader expands `~`, anchors relative paths to `~/.ssh/`, and handles basic `*`/`?` glob in the basename — covers `Include ~/.ssh/config.d/*` cleanly. Recursion capped at 16 levels to short-circuit cyclic includes. `Match` blocks are skipped (they don't introduce new Host names). - pipeToRemote() now serializes through a 2-slot semaphore. Five rapid Cmd+Shift+3s previously spawned five parallel ssh sessions; now at most two run concurrently with the rest queued. - Custom macOS screenshot prefix support: at startup, `defaults read com.apple.screencapture name` is queried and a second regex is built from the override (with metacharacter escaping). Users who customized their prefix via `defaults write … name "MyShot"` are now picked up. - loadConfig() now mtime-gates: stat first, return cached parse if mtimeMs matches the last successful load. The poll loop calls this 5x/sec; on the steady-state hot path it now does one stat instead of a full read+JSON.parse. saveConfig() invalidates the cache. +11 tests (Include, multi-spec Include, recursion cap, Match skip, isValidRemoteName, custom prefix regex). Total 37.
|
🎉 This PR is included in version 0.7.5 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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
New audit batch — five fixes (3× medium, 1× high, 1× low) bundled into one
fix:PR. Two flagged items were already shipped in Rounds H/I (Critical "remote home path" → fixed in v0.7.3 via `ssh echo $HOME` + cache; Low "config error spam" → already throttled in v0.7.3) — those are skipped here.-: a remote name like-oProxyCommand=evil.shwas passed unvalidated tospawn('ssh', [name, …]), where ssh would parse it as flags. Now `isValidRemoteName` rejects `-`-prefixed/whitespace/empty values at config-input time, and all three ssh spawn sites pass `--` before the remote as defense-in-depth.Includedirectives in~/.ssh/config: parseSSHConfig now follows them recursively with a loader pattern (testable). Resolves `/`, anchors relative paths to `/.ssh/`, supports basic ``/`?` glob — covers the canonical `Include ~/.ssh/config.d/` setup. Recursion capped at 16 levels. `Match` blocks are skipped.Test plan