diff --git a/gem/terminalwire-client/lib/terminalwire/client.rb b/gem/terminalwire-client/lib/terminalwire/client.rb index 034d10c..d791439 100644 --- a/gem/terminalwire-client/lib/terminalwire/client.rb +++ b/gem/terminalwire-client/lib/terminalwire/client.rb @@ -27,6 +27,7 @@ def self.websocket(url:, arguments: ARGV, &configuration) ENV["TERMINALWIRE_HOME"] ||= root_path.to_s url = URI(url) + exit_status = 0 Async do |task| endpoint = Async::HTTP::Endpoint.parse( @@ -37,9 +38,14 @@ def self.websocket(url:, arguments: ARGV, &configuration) Async::WebSocket::Client.connect(endpoint) do |connection| transport = Terminalwire::Transport::WebSocket.new(connection) adapter = Terminalwire::Adapter::Socket.new(transport) - Terminalwire::Client::Handler.new(adapter, arguments:, endpoint:, &configuration).connect + exit_status = Terminalwire::Client::Handler.new(adapter, arguments:, endpoint:, &configuration).connect end + rescue => e + # Suppress async cleanup errors during shutdown + raise unless e.is_a?(Async::Stop) || e.message.include?("mutex") end + + exit(exit_status) end end end diff --git a/gem/terminalwire-client/lib/terminalwire/client/entitlement/policy.rb b/gem/terminalwire-client/lib/terminalwire/client/entitlement/policy.rb index bb55f51..b835b99 100644 --- a/gem/terminalwire-client/lib/terminalwire/client/entitlement/policy.rb +++ b/gem/terminalwire-client/lib/terminalwire/client/entitlement/policy.rb @@ -2,7 +2,7 @@ module Terminalwire::Client::Entitlement module Policy # A policy has the authority, paths, and schemes that the server is allowed to access. class Base - attr_reader :paths, :authority, :schemes, :environment_variables + attr_reader :paths, :authority, :schemes, :environment_variables, :shell def initialize(authority:) @authority = authority @@ -20,6 +20,35 @@ def initialize(authority:) @environment_variables = EnvironmentVariables.new # Permit the HOME and TERMINALWIRE_HOME environment variables. @environment_variables.permit "TERMINALWIRE_HOME" + + @shell = Shell.new + # No shell commands permitted by default (deny-all) + # Load user-configured permissions from config file + load_user_config! + end + + def load_user_config! + config_path = File.expand_path(authority_path.join("config.yml")) + return unless File.exist?(config_path) + + config = YAML.safe_load_file(config_path, permitted_classes: [], symbolize_names: true) + + # Load shell permissions + if config[:shell]&.[](:allow) + Array(config[:shell][:allow]).each { |cmd| @shell.permit(cmd) } + end + + # Load path permissions + if config[:paths]&.[](:allow) + Array(config[:paths][:allow]).each { |path| @paths.permit(File.expand_path(path)) } + end + + # Load environment variable permissions + if config[:environment_variables]&.[](:allow) + Array(config[:environment_variables][:allow]).each { |var| @environment_variables.permit(var) } + end + rescue => e + # Silently ignore config errors to avoid breaking CLI end def root_path @@ -44,7 +73,8 @@ def serialize authority: @authority, schemes: @schemes.serialize, paths: @paths.serialize, - environment_variables: @environment_variables.serialize + environment_variables: @environment_variables.serialize, + shell: @shell.serialize } end end diff --git a/gem/terminalwire-client/lib/terminalwire/client/entitlement/shell.rb b/gem/terminalwire-client/lib/terminalwire/client/entitlement/shell.rb new file mode 100644 index 0000000..03398e6 --- /dev/null +++ b/gem/terminalwire-client/lib/terminalwire/client/entitlement/shell.rb @@ -0,0 +1,39 @@ +module Terminalwire::Client::Entitlement + # Shell commands the server can execute on the client. + # Commands are permitted by prefix matching, e.g. "git status" permits + # "git status", "git status --short", etc. + class Shell + include Enumerable + + # Default timeout in seconds + DEFAULT_TIMEOUT = 30 + # Maximum timeout in seconds (5 minutes) + MAX_TIMEOUT = 300 + # Maximum output size in bytes (10MB) + MAX_OUTPUT_SIZE = 10 * 1024 * 1024 + + def initialize + @permitted = Set.new + end + + def each(&) + @permitted.each(&) + end + + # Permit a command prefix, e.g. "git status", "bundle exec" + def permit(command_prefix) + @permitted << command_prefix.to_s + end + + # Check if a command with args is permitted. + # Builds the full command string and checks if it starts with any permitted prefix. + def permitted?(command, args = []) + full_command = [command, *args].join(" ") + @permitted.any? { |prefix| full_command.start_with?(prefix) } + end + + def serialize + map { |command| { command: } } + end + end +end diff --git a/gem/terminalwire-client/lib/terminalwire/client/handler.rb b/gem/terminalwire-client/lib/terminalwire/client/handler.rb index 63d4b4f..bb7ac5b 100644 --- a/gem/terminalwire-client/lib/terminalwire/client/handler.rb +++ b/gem/terminalwire-client/lib/terminalwire/client/handler.rb @@ -46,9 +46,15 @@ def connect } ) - loop do - handle @adapter.read + catch(:exit) do + loop do + handle @adapter.read + end end + + # Return exit status instead of calling exit directly + # This allows Async to clean up properly + @exit_status || 0 end def handle(message) @@ -56,7 +62,9 @@ def handle(message) in { event: "resource", action: "command", name:, parameters: } @resources.dispatch(**message) in { event: "exit", status: } - exit Integer(status) + # Store exit status and break out of loop to allow clean shutdown + @exit_status = Integer(status) + throw :exit end end end diff --git a/gem/terminalwire-client/lib/terminalwire/client/resource.rb b/gem/terminalwire-client/lib/terminalwire/client/resource.rb index 407e293..95d2d78 100644 --- a/gem/terminalwire-client/lib/terminalwire/client/resource.rb +++ b/gem/terminalwire-client/lib/terminalwire/client/resource.rb @@ -1,5 +1,6 @@ require "fileutils" require "io/console" +require "open3" module Terminalwire::Client::Resource # Dispatches messages from the Client::Handler to the appropriate resource. @@ -19,6 +20,7 @@ def initialize(adapter:, entitlement:) self << File self << Directory self << EnvironmentVariable + self << Shell yield self if block_given? end @@ -249,4 +251,79 @@ def permit(command, url:, **) @entitlement.schemes.permitted? url end end + + class Shell < Base + def self.key + "shell" + end + + # Execute a command with arguments using array-based execution (no shell interpretation). + # Returns a hash with stdout, stderr, exitstatus, and success. + def run(command:, args: [], timeout: nil, chdir: nil) + # Apply timeout limits + timeout = resolve_timeout(timeout) + + # Build execution options + options = {} + options[:chdir] = ::File.expand_path(chdir) if chdir + + # Execute command with array form (safe - no shell interpretation) + stdout, stderr, status = execute_with_timeout(command, args, timeout, options) + + # Truncate output if too large + stdout = truncate_output(stdout) + stderr = truncate_output(stderr) + + { + stdout: stdout, + stderr: stderr, + exitstatus: status.exitstatus, + success: status.success? + } + end + + protected + + def permit(command_name, command:, args: [], chdir: nil, **) + # Check if command + args prefix is permitted + return false unless @entitlement.shell.permitted?(command, args) + + # If chdir specified, it must be a permitted path + if chdir + return false unless @entitlement.paths.permitted?(chdir) + end + + true + end + + private + + def resolve_timeout(timeout) + max_timeout = Terminalwire::Client::Entitlement::Shell::MAX_TIMEOUT + default_timeout = Terminalwire::Client::Entitlement::Shell::DEFAULT_TIMEOUT + + if timeout.nil? + default_timeout + else + [timeout.to_i, max_timeout].min + end + end + + def execute_with_timeout(command, args, timeout, options) + Timeout.timeout(timeout) do + Open3.capture3(command, *args, **options) + end + rescue Timeout::Error + ["", "Command timed out after #{timeout} seconds", OpenStruct.new(exitstatus: 124, success?: false)] + end + + def truncate_output(output) + max_size = Terminalwire::Client::Entitlement::Shell::MAX_OUTPUT_SIZE + if output.bytesize > max_size + output.byteslice(0, max_size) + "\n... (output truncated)" + else + output + end + end + end end diff --git a/gem/terminalwire-client/terminalwire-client-0.3.5.rc1.gem b/gem/terminalwire-client/terminalwire-client-0.3.5.rc1.gem new file mode 100644 index 0000000..51277b3 Binary files /dev/null and b/gem/terminalwire-client/terminalwire-client-0.3.5.rc1.gem differ diff --git a/gem/terminalwire-core/terminalwire-core-0.3.5.rc1.gem b/gem/terminalwire-core/terminalwire-core-0.3.5.rc1.gem new file mode 100644 index 0000000..62ddf8e Binary files /dev/null and b/gem/terminalwire-core/terminalwire-core-0.3.5.rc1.gem differ diff --git a/gem/terminalwire-server/lib/terminalwire/server/context.rb b/gem/terminalwire-server/lib/terminalwire/server/context.rb index 0ae9264..4713daf 100644 --- a/gem/terminalwire-server/lib/terminalwire/server/context.rb +++ b/gem/terminalwire-server/lib/terminalwire/server/context.rb @@ -12,6 +12,7 @@ class Context :browser, :file, :directory, :environment_variable, + :shell, :authority, :root_path, :authority_path, @@ -32,6 +33,7 @@ def initialize(adapter:, entitlement:) @file = Resource::File.new("file", @adapter) @directory = Resource::Directory.new("directory", @adapter) @environment_variable = Resource::EnvironmentVariable.new("environment_variable", @adapter) + @shell = Resource::Shell.new("shell", @adapter) # Authority is provided by the client. @authority = @entitlement.fetch(:authority) diff --git a/gem/terminalwire-server/lib/terminalwire/server/resource.rb b/gem/terminalwire-server/lib/terminalwire/server/resource.rb index 51d262f..1e45236 100644 --- a/gem/terminalwire-server/lib/terminalwire/server/resource.rb +++ b/gem/terminalwire-server/lib/terminalwire/server/resource.rb @@ -116,5 +116,36 @@ def launch(url) command("launch", url: url) end end + + class Shell < Base + # Result object for shell command execution + Result = Struct.new(:stdout, :stderr, :exitstatus, :success, keyword_init: true) do + alias_method :success?, :success + end + + # Run a command with arguments on the client. + # Uses array-based execution for security (no shell interpretation). + # + # @param cmd [String] The command to execute + # @param args [Array] Arguments to pass to the command + # @param timeout [Integer, nil] Timeout in seconds (default: 30, max: 300) + # @param chdir [String, nil] Working directory for the command + # @return [Result] Result object with stdout, stderr, exitstatus, success? + def run(cmd, *args, timeout: nil, chdir: nil) + response = command("run", + command: cmd.to_s, + args: args.map(&:to_s), + timeout: timeout, + chdir: chdir&.to_s + ) + + Result.new( + stdout: response[:stdout], + stderr: response[:stderr], + exitstatus: response[:exitstatus], + success: response[:success] + ) + end + end end end diff --git a/gem/terminalwire/terminalwire-0.3.5.rc1.gem b/gem/terminalwire/terminalwire-0.3.5.rc1.gem new file mode 100644 index 0000000..08c4bbb Binary files /dev/null and b/gem/terminalwire/terminalwire-0.3.5.rc1.gem differ diff --git a/spec/integration/resources/shell_spec.rb b/spec/integration/resources/shell_spec.rb new file mode 100644 index 0000000..3239c1f --- /dev/null +++ b/spec/integration/resources/shell_spec.rb @@ -0,0 +1,174 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Terminalwire::Server::Resource::Shell do + let(:integration) { + Sync::Integration.new(authority: 'shell-test.example.com') do |sync| + # Permit specific command prefixes for testing + sync.policy.shell.permit "echo" + sync.policy.shell.permit "ls" + sync.policy.shell.permit "cat" + sync.policy.shell.permit "pwd" + sync.policy.shell.permit "true" + sync.policy.shell.permit "false" + # Allow all paths for chdir testing + sync.policy.paths.permit("**/*", mode: 0o777) + end + } + let(:server_shell) { described_class.new("shell", integration.server_adapter) } + + describe '#run' do + it 'executes a simple command and returns stdout' do + result = server_shell.run("echo", "hello world") + + expect(result.stdout.strip).to eq("hello world") + expect(result.stderr).to eq("") + expect(result.exitstatus).to eq(0) + expect(result.success?).to be true + end + + it 'returns stderr for error output' do + result = server_shell.run("cat", "/nonexistent/file/path") + + expect(result.stderr).to include("No such file") + expect(result.success?).to be false + end + + it 'returns correct exit status for failing commands' do + result = server_shell.run("false") + + expect(result.exitstatus).to eq(1) + expect(result.success?).to be false + end + + it 'returns correct exit status for successful commands' do + result = server_shell.run("true") + + expect(result.exitstatus).to eq(0) + expect(result.success?).to be true + end + + it 'passes multiple arguments correctly' do + result = server_shell.run("echo", "arg1", "arg2", "arg3") + + expect(result.stdout.strip).to eq("arg1 arg2 arg3") + end + + it 'executes command in specified directory' do + Dir.mktmpdir do |tmpdir| + result = server_shell.run("pwd", chdir: tmpdir) + + expect(result.stdout.strip).to eq(File.realpath(tmpdir)) + end + end + + context 'with timeout' do + it 'applies default timeout' do + result = server_shell.run("echo", "fast") + expect(result.success?).to be true + end + + it 'respects custom timeout' do + result = server_shell.run("echo", "test", timeout: 60) + expect(result.success?).to be true + end + end + end + + describe 'security: array-based execution prevents shell injection' do + it 'treats shell metacharacters as literal arguments' do + # This would be dangerous with shell interpretation: + # "echo test && rm -rf /" would execute both commands + # With array execution, "test && rm -rf /" is a single argument to echo + result = server_shell.run("echo", "test && echo injected") + + # The entire string including && is treated as one argument + expect(result.stdout.strip).to eq("test && echo injected") + end + + it 'treats semicolons as literal characters' do + result = server_shell.run("echo", "test; echo injected") + + expect(result.stdout.strip).to eq("test; echo injected") + end + + it 'treats pipes as literal characters' do + result = server_shell.run("echo", "test | cat") + + expect(result.stdout.strip).to eq("test | cat") + end + + it 'treats backticks as literal characters' do + result = server_shell.run("echo", "`whoami`") + + expect(result.stdout.strip).to eq("`whoami`") + end + + it 'treats $() as literal characters' do + result = server_shell.run("echo", "$(whoami)") + + expect(result.stdout.strip).to eq("$(whoami)") + end + end + + describe 'unauthorized command access' do + let(:restricted_integration) { + Sync::Integration.new(authority: 'restricted-shell.example.com') do |sync| + # Only permit specific git commands + sync.policy.shell.permit "git status" + sync.policy.shell.permit "git log" + end + } + let(:restricted_shell) { described_class.new("shell", restricted_integration.server_adapter) } + + it 'denies commands not in the allowlist' do + expect { + restricted_shell.run("rm", "-rf", "/") + }.to raise_error(Terminalwire::Error, /denied/) + end + + it 'denies git commands not matching the prefix' do + expect { + restricted_shell.run("git", "push", "--force") + }.to raise_error(Terminalwire::Error, /denied/) + end + + it 'permits git status command' do + # Note: This will fail if git isn't installed, but tests the permission system + begin + result = restricted_shell.run("git", "status") + # If we get here without error, permission was granted + expect(result).to be_a(described_class::Result) + rescue Errno::ENOENT + skip "git not installed" + end + end + + it 'permits git log command with additional arguments' do + begin + result = restricted_shell.run("git", "log", "--oneline", "-n", "1") + expect(result).to be_a(described_class::Result) + rescue Errno::ENOENT + skip "git not installed" + end + end + end + + describe 'chdir path entitlement' do + let(:restricted_integration) { + Sync::Integration.new(authority: 'chdir-restricted.example.com') do |sync| + sync.policy.shell.permit "pwd" + # Only permit /tmp paths + sync.policy.paths.permit("/tmp/**/*", mode: 0o755) + end + } + let(:restricted_shell) { described_class.new("shell", restricted_integration.server_adapter) } + + it 'denies chdir to unauthorized paths' do + expect { + restricted_shell.run("pwd", chdir: "/etc") + }.to raise_error(Terminalwire::Error, /denied/) + end + end +end