Skip to content

fix: round-L audit (parser comments, BSD pgrep, poll guard, log rotation)#21

Merged
flamerged merged 1 commit into
masterfrom
fix/audit-batch-l
May 1, 2026
Merged

fix: round-L audit (parser comments, BSD pgrep, poll guard, log rotation)#21
flamerged merged 1 commit into
masterfrom
fix/audit-batch-l

Conversation

@flamerged
Copy link
Copy Markdown
Owner

Summary

Six fixes from the latest audit, bundled into one `fix:` PR. Two flagged items were stale (Round-K test coverage for `buildCustomScreenshotRegex` + recursive includes was already added in v0.7.5).

  • MEDIUM — inline `#` comments parsed as aliases: `Host prod # primary` was emitting `prod`, `#`, `primary` as three separate aliases. Same hit `Include foo # note`. Fixed by stripping `(^|\s)#.*$` from each line, which mirrors OpenSSH's own rule (`#` only starts a comment when preceded by whitespace, so values like `prod#qa` are preserved).
  • HIGH — `pgrep -af` not BSD-portable: macOS BSD pgrep silently ignores `-a` (GNU extension), so the fallback discovery returned bare PIDs and lost the target-name extraction. Switched to `-lf` which works on both BSD and procps/Linux.
  • HIGH — poll-loop overlap: `setInterval(poll, 200ms)` could stack iterations on a slow ssh upload or hanging PowerShell, spawning duplicate clipboard reads + flooding the upload semaphore with the same hash. New `pollInFlight` guard self-throttles.
  • MEDIUM — daemon.log unbounded growth: opened append-only, never pruned. A recurring stderr (failing ssh, missing xclip) would grow it forever. Now rotates to `daemon.log.1` on next start when over 5 MB.
  • MEDIUM — clipboard write silent failures: `xclip`/`wl-copy`/`PowerShell`/`clip.exe` all return success-bool now; processNewImage logs "Clipboard write failed" honestly instead of erroneously "Copied to clipboard" on a non-zero exit.
  • MEDIUM — `isValidRemoteName` permissive: tightened to `[A-Za-z0-9.@][A-Za-z0-9.@-]*`. Defense-in-depth (no current interpolation path exists, but a future `sshshot logs ` feature would inherit the constraint).

Test plan

  • yarn typecheck / lint / build / format:check
  • yarn test — 41 (+4 new: parser comment-strip on Host, hash-in-token preserved, Include comment-strip, whitelist rejection of shell metacharacters)
  • CI matrix Node 20/22/24 green

…ion)

- parseSSHConfig now strips inline `# comment` from every directive line.
  `Host prod # primary` was treating `#` and `primary` as bogus aliases;
  `Include foo # note` was running 2-3 spurious filesystem lookups per
  cycle. The strip rule mirrors OpenSSH itself — `#` only starts a
  comment when preceded by whitespace, so values like `prod#qa` are
  preserved.
- pgrep -af → -lf for BSD compatibility. macOS BSD pgrep silently ignores
  -a (it's a GNU extension), which made the fallback discovery path emit
  PIDs without command lines and lose the daemon target name. -lf works
  on both BSD and GNU/procps.
- Poll-loop re-entrancy guard. setInterval(poll, 200ms) could stack
  iterations if a slow ssh upload or PowerShell call held one open past
  its deadline — concurrent xclip/PowerShell processes, semaphore
  flooded with the same image. A `pollInFlight` boolean now self-throttles
  the loop: long iterations just delay the next tick instead of stacking.
- daemon.log size-cap rotation. The file was opened append-only by
  startBackground and never pruned; a recurring stderr (failing ssh,
  missing xclip, etc.) would grow it forever. Now rotates to
  daemon.log.1 on next start when it exceeds 5 MB.
- Clipboard write status check. xclip/wl-copy/PowerShell/clip.exe all
  return success-bool now; processNewImage logs "Clipboard write failed"
  honestly instead of falsely reporting "Copied to clipboard" when the
  underlying tool exited non-zero (no X11 display, missing binary, etc).
- isValidRemoteName tightened to an explicit whitelist
  `[A-Za-z0-9._@][A-Za-z0-9._@-]*`. Defense-in-depth — no current code
  path interpolates `remote` into a remote shell, but a future feature
  that does (e.g. `sshshot logs <remote>`) would inherit the constraint.

+4 tests for the parser comment-stripping cases and the new whitelist
rejections (shell metacharacters). Total 41.
@flamerged flamerged merged commit cd3718f into master May 1, 2026
5 checks passed
@flamerged flamerged deleted the fix/audit-batch-l branch May 1, 2026 19:57
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 1, 2026

🎉 This PR is included in version 0.7.6 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant