Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ def index
scope = scope.where(agent_id: params[:agent_id]) if params[:agent_id].present?
scope = scope.where(status: params[:status]) if params[:status].present?

page = (params[:page] || 1).to_i
per_page = (params[:per_page] || 20).to_i
page = integer_param(:page, default: 1)
per_page = integer_param(:per_page, default: 20)
total = scope.count
runs = scope.offset((page - 1) * per_page).limit(per_page)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,9 @@ def restore
# observed from telemetry have no AgentRun rows at all, so a runs-only
# list showed them as empty while their scorecard reported real traffic.
def runs
minutes = params[:minutes].presence&.then { |m| m.to_i.clamp(1, 60 * 24 * 90) }
page = (params[:page] || 1).to_i
per_page = (params[:per_page] || 20).to_i
minutes = integer_param(:minutes)&.clamp(1, 60 * 24 * 90)
page = integer_param(:page, default: 1)
per_page = integer_param(:per_page, default: 20)

executions = AgentExecutions.new(
agents: [ @agent ],
Expand Down Expand Up @@ -282,7 +282,7 @@ def tool_roster
# with all-zero metrics beside a card and a runs list reporting real
# traffic.
def analytics
days = (params[:days] || 30).to_i
days = integer_param(:days, default: 30)
start_date = days.days.ago.beginning_of_day

runs = @agent.agent_runs.where("created_at >= ?", start_date)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ module Api
class AnalyticsController < BaseController
# GET /api/analytics
def index
days = (params[:days] || 30).to_i
days = integer_param(:days, default: 30)
start_date = days.days.ago.beginning_of_day

# Table names are interpolated rather than written literally: the
Expand Down
20 changes: 20 additions & 0 deletions actionagent/app/controllers/action_agent/api/base_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,26 @@ def require_execution_enabled!
render json: { error: "Agent execution is disabled on this dashboard" }, status: :forbidden
end

# An integer query param. A value can arrive as a container
# (`minutes[]=1&minutes[]=2`, or `page[x]=1`), and neither Array nor
# ActionController::Parameters responds to `to_i`: reading them
# directly raised NoMethodError and turned a malformed query into a
# 500. A multi-valued param means its first value; anything else that
# is not a scalar falls back to the default.
def integer_param(name, default: nil)
raw = params[name]
raw = raw.first if raw.is_a?(Array)
return default if raw.blank? || !(raw.is_a?(String) || raw.is_a?(Numeric))

raw.to_s.to_i
end

# integer_param, then clamped into [min, max]. Non-numeric input becomes
# 0 and is then clamped up to `min`.
def clamped_param(name, default:, min:, max:)
integer_param(name, default: default).clamp(min, max)
end

def not_found
render json: { error: "Record not found" }, status: :not_found
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class InteractionsController < BaseController

# GET /api/interactions
def index
limit = params.fetch(:limit, DEFAULT_LIMIT).to_i.clamp(1, 200)
limit = clamped_param(:limit, default: DEFAULT_LIMIT, min: 1, max: 200)

contexts = interactions_scope
.includes(:contextable)
Expand Down Expand Up @@ -140,8 +140,8 @@ def traces_for_agent(agent_id)
def window_minutes
return @window_minutes if defined?(@window_minutes)

raw = params[:minutes].presence
@window_minutes = raw ? raw.to_i.clamp(1, MAX_WINDOW_MINUTES) : nil
raw = integer_param(:minutes)
@window_minutes = raw ? raw.clamp(1, MAX_WINDOW_MINUTES) : nil
end

def interactions_scope
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,16 @@ class SandboxesController < BaseController
# POST /api/sandboxes/compare
# Run multiple providers in a single sandbox using parallel generation jobs
def compare
providers = params[:providers] || %w[anthropic openai ollama]
providers = params[:providers].nil? ? %w[anthropic openai ollama] : params[:providers]
task = params[:task]
sandbox_id = params[:sandbox_id]

return render json: { error: "Task required" }, status: :bad_request unless task.present?
# A bare string or a nested object is a malformed request, not a list
# of one provider: reading it as a list raised NoMethodError.
unless providers.is_a?(Array) && providers.all? { |name| name.is_a?(String) }
return render json: { error: "providers must be a list of provider names" }, status: :bad_request
end
return render json: { error: "At least 2 providers required" }, status: :bad_request if providers.size < 2

# Validate providers
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@ class SessionRecordingsController < BaseController

before_action :set_recording, only: [ :show, :actions, :snapshot, :export, :handoff ]

# Browser state that must never leave the server in a read response:
# the handoff state a recording carries is a copy of the visitor's
# cookies and web storage. Only #handoff returns it, to the owner, when
# they continue the session.
SENSITIVE_STATE_KEYS = %w[cookies session_storage local_storage].freeze

# GET /api/session_recordings
# List recordings with optional filters
def index
Expand All @@ -36,8 +42,8 @@ def index
end

# Pagination
page = (params[:page] || 1).to_i
per_page = [ (params[:per_page] || 20).to_i, 100 ].min
page = integer_param(:page, default: 1)
per_page = [ integer_param(:per_page, default: 20), 100 ].min
offset = (page - 1) * per_page

total = recordings.count
Expand Down Expand Up @@ -79,10 +85,10 @@ def actions

# Support pagination for large recordings
if params[:after_sequence].present?
actions = actions.where("sequence > ?", params[:after_sequence].to_i)
actions = actions.where("sequence > ?", integer_param(:after_sequence, default: 0))
end

limit = [ params[:limit]&.to_i || 100, 500 ].min
limit = [ integer_param(:limit, default: 100), 500 ].min
actions = actions.limit(limit)

render json: {
Expand Down Expand Up @@ -332,7 +338,7 @@ def recording_detail(recording)
created_at: recording.created_at.iso8601,
updated_at: recording.updated_at.iso8601,
timeline: recording.timeline,
handoff_state: recording.metadata["handoff_state"],
handoff_state: safe_handoff_state(recording.metadata["handoff_state"]),
agent: recording.agent_run&.agent&.slice(:id, :name),
sandbox_session: recording.sandbox_session&.summary
}
Expand All @@ -343,9 +349,20 @@ def first_screenshot_url(recording)
action&.screenshot_url(expires_in: 1.hour)
end

# Strips the browser state at the top level and inside handoff_state,
# which the model stores nested (a recording's metadata carries the
# handoff as one key), so a show response never ships a session cookie.
def safe_metadata(metadata)
# Remove sensitive data from metadata
metadata.except("cookies", "session_storage", "local_storage")
safe = (metadata || {}).except(*SENSITIVE_STATE_KEYS)
return safe unless safe["handoff_state"].is_a?(Hash)

safe.merge("handoff_state" => safe_handoff_state(safe["handoff_state"]))
end

def safe_handoff_state(handoff_state)
return handoff_state unless handoff_state.is_a?(Hash)

handoff_state.except(*SENSITIVE_STATE_KEYS)
end

def generate_visitor_id
Expand Down
66 changes: 66 additions & 0 deletions actionagent/test/param_coercion_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
# frozen_string_literal: true

require "test_helper"

# A query value can arrive as a container (`minutes[]=1&minutes[]=2`, or
# `page[x]=1`), and neither Array nor ActionController::Parameters responds
# to `to_i`. Reading them directly raised NoMethodError and turned a
# malformed query into a 500 on every list the dashboard paginates or
# windows. A multi-valued param means its first value; a nested object is
# malformed and floors to the default.
class ParamCoercionTest < ActionDispatch::IntegrationTest
def setup
ActionAgent::Agent.delete_all
@agent = ActionAgent::Agent.create!(name: "Support", provider: "openai", model: "gpt-4o-mini")
@agent.agent_runs.create!(input_prompt: "hi", output: "hello", status: :complete)
end

test "agent runs coerce container-valued minutes, page and per_page" do
get "/activeagents/api/agents/#{@agent.id}/runs", params: { minutes: [ 1, 2 ], page: { x: 1 }, per_page: [ 5 ] }

assert_response :success, response.body
body = JSON.parse(response.body)
assert_equal 1, body["runs"].length
assert_equal 5, body["meta"]["per_page"]
assert_equal 1, body["meta"]["page"]
end

test "agent analytics coerces a container-valued days param" do
get "/activeagents/api/agents/#{@agent.id}/analytics", params: { days: [ 7, 30 ] }

assert_response :success, response.body
end

test "interactions coerce container-valued minutes and limit" do
context = ActionAgent::AgentContext.create!(contextable: @agent, agent_name: "SupportAgent", action_name: "respond")
context.add_user_message("Where is order 88213?")

get "/activeagents/api/interactions", params: { minutes: [ 60, 120 ], limit: { n: 10 } }

assert_response :success, response.body
assert_equal 1, JSON.parse(response.body)["interactions"].length
end

test "session recordings coerce container-valued page, per_page, after_sequence and limit" do
recording = ActionAgent::SessionRecording.start_user_session!(page_url: "https://example.com/")
recording.record_action!(action_type: "click", selector: "button")

get "/activeagents/api/session_recordings", params: { page: [ 1 ], per_page: { n: 20 } }
assert_response :success, response.body
assert_equal 1, JSON.parse(response.body).dig("pagination", "page")

get "/activeagents/api/session_recordings/#{recording.id}/actions", params: { after_sequence: [ 0 ], limit: { n: 5 } }
assert_response :success, response.body
assert_equal 1, JSON.parse(response.body)["actions"].size
end

test "sandbox compare rejects a providers value that is not a list of names" do
sandbox = ActionAgent::SandboxSession.create!(session_id: SecureRandom.uuid, status: :ready, expires_at: 1.hour.from_now)

post "/activeagents/api/sandboxes/compare", params: { task: "Take a screenshot", providers: "anthropic", sandbox_id: sandbox.session_id }
assert_response :bad_request, response.body

post "/activeagents/api/sandboxes/compare", params: { task: "Take a screenshot", providers: { a: "anthropic" }, sandbox_id: sandbox.session_id }
assert_response :bad_request, response.body
end
end
23 changes: 23 additions & 0 deletions actionagent/test/session_recording_privacy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,29 @@ def setup
assert_nil exported["metadata"]["password"]
assert_not_includes response.body, "hunter2secret"
end

# The handoff state is a copy of the visitor's browser: cookies and web
# storage. Stripping only the top level of the metadata left the same
# secrets readable one key down, and as the top-level handoff_state key.
test "show strips cookies and web storage from the nested handoff state" do
@recording.update!(metadata: @recording.metadata.merge(
"handoff_state" => {
"url" => "https://example.com/checkout",
"cookies" => [ { "name" => "_session", "value" => "sekrit-cookie" } ],
"local_storage" => { "auth_token" => "lst-secret" },
"session_storage" => { "csrf" => "sst-secret" }
}
))

get "/activeagents/api/session_recordings/#{@recording.id}"

assert_response :success
body = JSON.parse(response.body)["recording"]
assert_equal "https://example.com/checkout", body.dig("handoff_state", "url")
assert_nil body.dig("handoff_state", "cookies")
assert_nil body.dig("metadata", "handoff_state", "cookies")
%w[sekrit-cookie lst-secret sst-secret].each { |secret| assert_not_includes response.body, secret }
end
end

# In a per-user install the list has to show the recordings the caller can
Expand Down
Loading