Harden alpha8 publication safety
This commit is contained in:
@@ -9,6 +9,12 @@
|
||||
# rename work.
|
||||
|
||||
require "opencode-ruby"
|
||||
require "fileutils"
|
||||
require "marcel"
|
||||
require "pathname"
|
||||
require "set"
|
||||
require "stringio"
|
||||
require "tempfile"
|
||||
|
||||
require "active_support/core_ext/object/blank" # blank?, present?, presence
|
||||
require "active_support/core_ext/object/try"
|
||||
|
||||
@@ -80,6 +80,10 @@ module Opencode
|
||||
|
||||
if (transform = transforms.find { |t| t.applies_to?(file) })
|
||||
attached += 1 if apply_transform(transform, file)
|
||||
elsif transform_owned_filenames.include?(file.basename)
|
||||
# Destination names belong exclusively to their host transform.
|
||||
# Agent-authored bytes must never fall through as trusted output.
|
||||
next
|
||||
elsif default_attach == :all
|
||||
# Default identity path: every safe sandbox file that no
|
||||
# transform claims attaches as-is. Callers that want the
|
||||
|
||||
@@ -15,6 +15,8 @@ module Opencode
|
||||
# not here — the file doesn't know which turn opened "after." That's
|
||||
# a property of the scan, not a property of the file.
|
||||
class SandboxFile
|
||||
UnsafeFileError = Class.new(Opencode::Error)
|
||||
|
||||
attr_reader :path, :sandbox_prefix
|
||||
|
||||
def initialize(path:, sandbox_prefix:, max_bytes:)
|
||||
@@ -36,7 +38,13 @@ module Opencode
|
||||
end
|
||||
|
||||
def content
|
||||
File.read(path)
|
||||
with_safe_file do |file|
|
||||
content = file.read(@max_bytes + 1) || "".b
|
||||
if content.bytesize > @max_bytes
|
||||
raise UnsafeFileError, "Sandbox file exceeds size limit while reading: #{basename}"
|
||||
end
|
||||
content
|
||||
end
|
||||
end
|
||||
|
||||
def content_type
|
||||
@@ -52,17 +60,11 @@ module Opencode
|
||||
# - Reject anything over the size cap (default
|
||||
# Opencode::ResponseParser::MAX_ARTIFACT_SIZE = 10 MB).
|
||||
#
|
||||
# The Sandbox scan filters non-files (directories, FIFOs) before
|
||||
# yielding, so we don't re-check #file? here.
|
||||
# Revalidates the opened file descriptor so a path swap between the
|
||||
# sandbox scan and the read cannot redirect content outside the sandbox.
|
||||
def safe?
|
||||
return false if File.symlink?(path)
|
||||
return false unless Pathname.new(path).realpath.to_s.start_with?(sandbox_prefix)
|
||||
return false if size > @max_bytes
|
||||
|
||||
true
|
||||
rescue Errno::ENOENT
|
||||
# Concurrent deletion between scan-yield and safety-check — treat
|
||||
# as unsafe so the orchestrator skips rather than crashing.
|
||||
with_safe_file { true }
|
||||
rescue UnsafeFileError
|
||||
false
|
||||
end
|
||||
|
||||
@@ -77,5 +79,31 @@ module Opencode
|
||||
content_type: content_type
|
||||
)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def with_safe_file
|
||||
before = File.lstat(path)
|
||||
raise UnsafeFileError, "Unsafe sandbox file: #{basename}" unless before.file? && !before.symlink?
|
||||
|
||||
resolved = Pathname.new(path).realpath.to_s
|
||||
unless resolved.start_with?(sandbox_prefix)
|
||||
raise UnsafeFileError, "Sandbox file escapes its root: #{basename}"
|
||||
end
|
||||
|
||||
File.open(path, "rb") do |file|
|
||||
opened = file.stat
|
||||
unless opened.file? && opened.dev == before.dev && opened.ino == before.ino
|
||||
raise UnsafeFileError, "Sandbox file changed while opening: #{basename}"
|
||||
end
|
||||
if opened.size > @max_bytes
|
||||
raise UnsafeFileError, "Sandbox file exceeds size limit: #{basename}"
|
||||
end
|
||||
|
||||
yield file
|
||||
end
|
||||
rescue Errno::ENOENT, Errno::ELOOP => e
|
||||
raise UnsafeFileError, "Unsafe sandbox file #{basename}: #{e.message}"
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -31,7 +31,8 @@ module Opencode
|
||||
# trusted?(attachment) — true if the attachment was produced by
|
||||
# this transform (used by Impostor.for and
|
||||
# by view code that decides inline-render
|
||||
# vs download). Default: filename match.
|
||||
# vs download). Default: false; subclasses
|
||||
# must verify host-authored metadata.
|
||||
# purge_impostors? — if true, before attaching the substrate
|
||||
# deletes any existing attachment whose
|
||||
# filename matches destination_filename
|
||||
@@ -60,8 +61,8 @@ module Opencode
|
||||
raise NotImplementedError, "#{self.class.name} must implement #render"
|
||||
end
|
||||
|
||||
def trusted?(attachment)
|
||||
attachment.filename.to_s == destination_filename
|
||||
def trusted?(_attachment)
|
||||
false
|
||||
end
|
||||
|
||||
def purge_impostors?
|
||||
|
||||
@@ -24,8 +24,10 @@ module Opencode
|
||||
# Side effect, unchanged from the concern: file bytes are copied from
|
||||
# ActiveStorage into the per-user OpenCode sandbox directory so the
|
||||
# agent can read them with the `read` tool. The copy is path-escape
|
||||
# guarded (the cleanpath of the destination must start with the
|
||||
# sandbox dir prefix, no symlink trickery).
|
||||
# guarded: generated names must be basenames, and all writes stay anchored
|
||||
# to an opened directory handle even if the sandbox path is replaced.
|
||||
# A temporary file is renamed into place so destination symlinks are
|
||||
# replaced rather than followed. Retried jobs can refresh existing copies.
|
||||
class UploadedFilesPrompt
|
||||
attr_reader :text, :sandbox_file_names
|
||||
|
||||
@@ -52,24 +54,77 @@ module Opencode
|
||||
[
|
||||
raw,
|
||||
"",
|
||||
"The user uploaded #{file_instructions.size} file(s). Read each file thoroughly, then consult your reference materials and verify any legal claims before responding:",
|
||||
"The user uploaded #{file_instructions.size} file(s). Read each file thoroughly before responding:",
|
||||
*file_instructions
|
||||
].join("\n").strip
|
||||
end
|
||||
|
||||
def copy_to_sandbox(file)
|
||||
FileUtils.mkdir_p(@sandbox_path)
|
||||
sandbox_path = Pathname.new(@sandbox_path).expand_path
|
||||
FileUtils.mkdir_p(sandbox_path)
|
||||
sandbox_stat = File.lstat(sandbox_path)
|
||||
unless sandbox_stat.directory? && !sandbox_stat.symlink?
|
||||
raise ArgumentError, "Sandbox root must be a directory, not a symlink: #{sandbox_path}"
|
||||
end
|
||||
|
||||
sandbox_name = @sandbox_name_for.call(file)
|
||||
dest = File.join(@sandbox_path, sandbox_name)
|
||||
|
||||
resolved = Pathname.new(dest).cleanpath.to_s
|
||||
unless resolved.start_with?(@sandbox_path)
|
||||
sandbox_name = @sandbox_name_for.call(file).to_s
|
||||
unless sandbox_name == File.basename(sandbox_name) && !%w[. ..].include?(sandbox_name)
|
||||
raise ArgumentError, "Filename escapes sandbox: #{sandbox_name}"
|
||||
end
|
||||
|
||||
File.open(dest, "wb") { |f| f.write(file.download) }
|
||||
Placement.new(sandbox_name, dest)
|
||||
File.open(sandbox_path, File::RDONLY) do |directory|
|
||||
opened_stat = directory.stat
|
||||
unless opened_stat.directory? && same_file?(sandbox_stat, opened_stat)
|
||||
raise ArgumentError, "Sandbox root changed while opening: #{sandbox_path}"
|
||||
end
|
||||
|
||||
directory_path = directory_handle_path(directory)
|
||||
dest = File.join(directory_path, sandbox_name)
|
||||
mode = destination_mode(dest)
|
||||
|
||||
Tempfile.create([ ".opencode-upload-", ".tmp" ], directory_path) do |temp|
|
||||
temp.binmode
|
||||
temp.write(file.download)
|
||||
temp.flush
|
||||
temp.chmod(mode)
|
||||
File.rename(temp.path, dest)
|
||||
|
||||
unless same_file_at_path?(sandbox_path, opened_stat)
|
||||
File.unlink(dest)
|
||||
raise ArgumentError, "Sandbox root changed during upload: #{sandbox_path}"
|
||||
end
|
||||
end
|
||||
end
|
||||
Placement.new(sandbox_name, sandbox_path.join(sandbox_name).to_s)
|
||||
end
|
||||
|
||||
def directory_handle_path(directory)
|
||||
stat = directory.stat
|
||||
[ "/proc/self/fd/#{directory.fileno}", "/dev/fd/#{directory.fileno}" ].find do |path|
|
||||
same_file?(File.stat(path), stat)
|
||||
rescue Errno::ENOENT
|
||||
false
|
||||
end || raise(ArgumentError, "Platform cannot anchor writes to the opened sandbox directory")
|
||||
end
|
||||
|
||||
def destination_mode(path)
|
||||
stat = File.lstat(path)
|
||||
return stat.mode & 0o777 if stat.file? && !stat.symlink?
|
||||
|
||||
0o666 & ~File.umask
|
||||
rescue Errno::ENOENT
|
||||
0o666 & ~File.umask
|
||||
end
|
||||
|
||||
def same_file_at_path?(path, expected)
|
||||
current = File.lstat(path)
|
||||
current.directory? == expected.directory? && !current.symlink? && same_file?(current, expected)
|
||||
rescue Errno::ENOENT
|
||||
false
|
||||
end
|
||||
|
||||
def same_file?(left, right)
|
||||
left.dev == right.dev && left.ino == right.ino
|
||||
end
|
||||
|
||||
# Tiny value pair returned by copy_to_sandbox: the canonical filename
|
||||
|
||||
Reference in New Issue
Block a user