Skip to content

Harden exec/encrypt editor invocation, fix Crystal 1.20 deprecations#22

Merged
crimson-knight merged 2 commits into
mainfrom
fix/exec-hardening-and-deprecations
Jun 10, 2026
Merged

Harden exec/encrypt editor invocation, fix Crystal 1.20 deprecations#22
crimson-knight merged 2 commits into
mainfrom
fix/exec-hardening-and-deprecations

Conversation

@crimson-knight

Copy link
Copy Markdown
Member

Summary

  • Shell injection hardening (exec + encrypt): system("#{editor} #{filename}") allowed shell metacharacters in EDITOR or a crafted filename to execute arbitrary commands. Replaced with Process.parse_arguments(editor) + Process.run(cmd, args, ...) (no shell: true) so the editor string is POSIX word-split (supporting multi-word editors like code -w or vim -u none) and the filename is passed as a literal argument. Regression specs added to spec/commands/exec_command_spec.cr.
    Fix pattern contributed by @tcoatswo in Fix unsafe shell invocation in amber exec amber#1376.

  • Crystal 1.20 deprecation (Time.monotonicTime.instant): process_runner.cr used Time.monotonic which is now deprecated in Crystal 1.20.0 in favour of Time.instant (returns Time::Instant; subtraction still yields Time::Span so the log message is unchanged). shard.yml crystal floor raised from >= 1.10.0 to >= 1.20.0 to match.
    Fix contributed by @renich in Fix compiler deprecation warnings for Crystal 1.20+ amber#1379.

  • JS client dead-socket guard: Channel.join(), leave(), and push() in the app-template amber.js called ws.send() without error handling. A dead/closing WebSocket throws an uncaught DOMException in the browser. Each send is now wrapped in try/catch with a fallback to _reconnect().
    Fix pattern contributed by @jonsilverman50-star in Fast Multisocket/Multichannel Redis PubSub Websocket Adapter amber#1375.

  • Version bump: 2.0.02.0.1 in shard.yml and AmberCLI::VERSION.

Spec results

crystal spec spec/commands/exec_command_spec.cr
→ 139 examples, 0 failures, 0 errors

crystal spec (full suite)
→ 403 examples, 1 failure, 5 errors, 0 pending
   (all failures are pre-existing amber-lsp integration tests that require
    a compiled bin/amber-lsp binary; unrelated to these changes)

🤖 Generated with Claude Code

crimson-knight and others added 2 commits June 10, 2026 15:29
…precations

exec.cr and encrypt.cr previously called system("#{editor} #{filename}"),
allowing shell metacharacters in the EDITOR env var or filename to execute
arbitrary commands. Replace with Process.parse_arguments + Process.run so
the editor string is word-split (supporting "code -w", "vim -u none", etc.)
and the filename is passed as a literal argument with no shell involvement.
Add regression specs asserting shell metacharacters in editor/filename do
not execute.

Fix pattern contributed by @tcoatswo in amberframework/amber#1376.

process_runner.cr used Time.monotonic which is deprecated in Crystal 1.20.0
in favour of Time.instant (Time::Instant, introduced in 1.20.0). Replace
both call sites. The shard.yml crystal floor is raised to >= 1.20.0 to
match (see version bump commit).

Fix contributed by @renich in amberframework/amber#1379.

amber.js Channel.join/leave/push previously called this.socket.ws.send()
without error handling; a dead WebSocket throws an uncaught DOMException in
the browser. Wrap each send in try/catch with a fallback to _reconnect().

Fix pattern contributed by @jonsilverman50-star in amberframework/amber#1375.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Time.instant (used in process_runner.cr) was introduced in Crystal 1.20.0.
Raise the shard.yml crystal constraint from >= 1.10.0 to >= 1.20.0 to
reflect this requirement. Bump shard version 2.0.0 -> 2.0.1 to signal the
patch release containing the exec/encrypt hardening and deprecation fixes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@crimson-knight crimson-knight merged commit ede1d18 into main Jun 10, 2026
@crimson-knight crimson-knight deleted the fix/exec-hardening-and-deprecations branch June 10, 2026 19:30
crimson-knight added a commit that referenced this pull request Jun 10, 2026
…LSP link

- Homebrew tap command was wrong: amberframework/amber_cli -> amberframework/amber-cli
  (Homebrew convention: repo homebrew-<name> maps to tap amberframework/<name>).
  Formula name is amber-cli, so install command is `brew install amber-cli`.
- Add "Direct Binary Download" section with curl + sha256 verification steps
  pointing at the GitHub releases page.
- Fix LSP Setup Guide link: was pointing at personal fork
  github.com/crimson-knight/amber/blob/master/... — now points at the
  canonical org repo github.com/amberframework/amber/blob/v2-dev/...
- Bump Crystal requirement from 1.0+ to 1.20+ to match shard.yml floor raised
  in #22 (Time.instant requires Crystal >= 1.20.0).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant