Creepso/agent-desktop
1
0
Fork 0
mirror of https://github.com/lahfir/agent-desktop.git synced 2026-08-19 21:49:23 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix: centralize ref preflight validation

Browse source
This commit is contained in:
Lahfir 2026-05-19 17:58:37 -07:00
parent e1075566cb
commit 2d3929b59e
8 changed files with 84 additions and 85 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
crates/core/src/commands/helpers.rs
View file

@ -3,7 +3,7 @@ use crate::{
adapter::{PlatformAdapter, WindowFilter},
error::AppError,
node::WindowInfo,
refs::RefEntry,
refs::{RefEntry, validate_ref_id},
refs_store::RefStore,
resolved_element::ResolvedElement,
window_lookup,
@ -46,20 +46,6 @@ pub(crate) fn resolve_ref<'a>(
Ok((entry, ResolvedElement::new(adapter, handle)))
}
pub(crate) fn validate_ref_id(ref_id: &str) -> Result<(), AppError> {
let valid = ref_id.starts_with("@e")
&& ref_id.len() >= 3
&& ref_id.len() <= 12
&& ref_id[2..].chars().all(|c| c.is_ascii_digit())
&& ref_id[2..].parse::<u32>().is_ok_and(|n| n > 0);
if !valid {
return Err(AppError::invalid_input(format!(
"Invalid ref_id '{ref_id}': must match @e{{N}} where N is a positive integer"
)));
}
Ok(())
}
pub(crate) fn resolve_app_pid(
app: Option<&str>,
adapter: &dyn PlatformAdapter,

18
crates/core/src/commands/helpers_tests.rs
View file

@ -149,21 +149,3 @@ fn restore_can_run_when_no_window_is_currently_listed() {
assert_eq!(value["restored"], true);
assert_eq!(adapter.op_count.load(Ordering::SeqCst), 1);
}
#[test]
fn test_valid_refs() {
assert!(validate_ref_id("@e1").is_ok());
assert!(validate_ref_id("@e14").is_ok());
assert!(validate_ref_id("@e999").is_ok());
}
#[test]
fn test_invalid_refs() {
assert!(validate_ref_id("@").is_err());
assert!(validate_ref_id("e1").is_err());
assert!(validate_ref_id("@e").is_err());
assert!(validate_ref_id("@e0").is_err());
assert!(validate_ref_id("@e0abc").is_err());
assert!(validate_ref_id("1").is_err());
assert!(validate_ref_id("").is_err());
}

2
crates/core/src/commands/snapshot.rs
View file

@ -1,7 +1,7 @@
use crate::{
adapter::{PlatformAdapter, SnapshotSurface},
commands::helpers::validate_ref_id,
error::AppError,
refs::validate_ref_id,
snapshot, snapshot_ref,
};
use serde_json::{Value, json};

4
crates/core/src/commands/wait.rs
View file

@ -1,10 +1,10 @@
use crate::{
adapter::{PlatformAdapter, WindowFilter},
commands::{helpers::resolve_app_pid, helpers::validate_ref_id},
commands::helpers::resolve_app_pid,
error::{AppError, ErrorCode},
node::AccessibilityNode,
notification::NotificationFilter,
refs::RefMap,
refs::{RefMap, validate_ref_id},
refs_store::RefStore,
search_text, snapshot,
};

14
crates/core/src/refs.rs
View file

@ -51,6 +51,20 @@ fn is_false(value: &bool) -> bool {
!*value
}
pub fn validate_ref_id(ref_id: &str) -> Result<(), AppError> {
let valid = ref_id.starts_with("@e")
&& ref_id.len() >= 3
&& ref_id.len() <= 12
&& ref_id[2..].chars().all(|c| c.is_ascii_digit())
&& ref_id[2..].parse::<u32>().is_ok_and(|n| n > 0);
if valid {
return Ok(());
}
Err(AppError::invalid_input(format!(
"Invalid ref_id '{ref_id}': must match @e{{N}} where N is a positive integer"
)))
}
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct RefMap {
inner: HashMap<String, RefEntry>,

18
crates/core/src/refs_tests.rs
View file

@ -62,6 +62,24 @@ fn test_get_missing() {
assert!(map.get("@e99").is_none());
}
#[test]
fn test_validate_ref_id_accepts_positive_element_refs() {
assert!(validate_ref_id("@e1").is_ok());
assert!(validate_ref_id("@e14").is_ok());
assert!(validate_ref_id("@e999").is_ok());
}
#[test]
fn test_validate_ref_id_rejects_malformed_refs() {
assert!(validate_ref_id("@").is_err());
assert!(validate_ref_id("e1").is_err());
assert!(validate_ref_id("@e").is_err());
assert!(validate_ref_id("@e0").is_err());
assert!(validate_ref_id("@e0abc").is_err());
assert!(validate_ref_id("1").is_err());
assert!(validate_ref_id("").is_err());
}
#[test]
fn test_remove_by_root_ref() {
let mut map = RefMap::new();

93
src/command_policy.rs
View file

@ -1,13 +1,10 @@
use agent_desktop_core::{
PermissionReport,
error::{AdapterError, AppError, ErrorCode},
refs::validate_ref_id,
};
use crate::cli::Commands;
use crate::dispatch_parse::{
parse_direction, parse_get_property, parse_is_property, parse_mouse_button, parse_xy,
parse_xy_opt,
};
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) enum PermissionNeed {
@ -117,16 +114,14 @@ fn validate_args(cmd: &Commands) -> Result<(), AppError> {
"--root cannot be combined with --surface",
));
}
validate_cli_ref_id(root)?;
validate_ref_id(root)?;
}
}
Commands::Get(args) => {
validate_cli_ref_id(&args.ref_id)?;
parse_get_property(&args.property)?;
validate_ref_id(&args.ref_id)?;
}
Commands::Is(args) => {
validate_cli_ref_id(&args.ref_id)?;
parse_is_property(&args.property)?;
validate_ref_id(&args.ref_id)?;
}
Commands::Click(args)
| Commands::DoubleClick(args)
@ -140,66 +135,68 @@ fn validate_args(cmd: &Commands) -> Result<(), AppError> {
| Commands::Expand(args)
| Commands::Collapse(args)
| Commands::ScrollTo(args) => {
validate_cli_ref_id(&args.ref_id)?;
validate_ref_id(&args.ref_id)?;
}
Commands::Type(args) => validate_cli_ref_id(&args.ref_id)?,
Commands::SetValue(args) => validate_cli_ref_id(&args.ref_id)?,
Commands::Select(args) => validate_cli_ref_id(&args.ref_id)?,
Commands::Type(args) => validate_ref_id(&args.ref_id)?,
Commands::SetValue(args) => validate_ref_id(&args.ref_id)?,
Commands::Select(args) => validate_ref_id(&args.ref_id)?,
Commands::Scroll(args) => {
validate_cli_ref_id(&args.ref_id)?;
parse_direction(&args.direction)?;
validate_ref_id(&args.ref_id)?;
}
Commands::Hover(args) => {
if let Some(ref_id) = &args.ref_id {
validate_cli_ref_id(ref_id)?;
validate_ref_id(ref_id)?;
}
parse_xy_opt(args.xy.as_deref())?;
}
Commands::Drag(args) => {
if let Some(ref_id) = &args.from {
validate_cli_ref_id(ref_id)?;
validate_ref_id(ref_id)?;
}
if let Some(ref_id) = &args.to {
validate_cli_ref_id(ref_id)?;
validate_ref_id(ref_id)?;
}
parse_xy_opt(args.from_xy.as_deref())?;
parse_xy_opt(args.to_xy.as_deref())?;
}
Commands::MouseMove(args) => {
parse_xy(&args.xy)?;
}
Commands::MouseClick(args) => {
parse_xy(&args.xy)?;
parse_mouse_button(&args.button)?;
}
Commands::MouseDown(args) | Commands::MouseUp(args) => {
parse_xy(&args.xy)?;
parse_mouse_button(&args.button)?;
}
Commands::Wait(args) => {
if let Some(ref_id) = &args.element {
validate_cli_ref_id(ref_id)?;
validate_ref_id(ref_id)?;
}
}
_ => {}
Commands::Find(_)
| Commands::Screenshot(_)
| Commands::Press(_)
| Commands::KeyDown(_)
| Commands::KeyUp(_)
| Commands::MouseMove(_)
| Commands::MouseClick(_)
| Commands::MouseDown(_)
| Commands::MouseUp(_)
| Commands::Launch(_)
| Commands::CloseApp(_)
| Commands::ListWindows(_)
| Commands::ListApps(_)
| Commands::FocusWindow(_)
| Commands::ResizeWindow(_)
| Commands::MoveWindow(_)
| Commands::Minimize(_)
| Commands::Maximize(_)
| Commands::Restore(_)
| Commands::ListSurfaces(_)
| Commands::ListNotifications(_)
| Commands::DismissNotification(_)
| Commands::DismissAllNotifications(_)
| Commands::NotificationAction(_)
| Commands::ClipboardGet
| Commands::ClipboardSet(_)
| Commands::ClipboardClear
| Commands::Status
| Commands::Permissions(_)
| Commands::Version(_)
| Commands::Batch(_)
| Commands::Skills(_) => {}
}
Ok(())
}
fn validate_cli_ref_id(ref_id: &str) -> Result<(), AppError> {
let valid = ref_id.starts_with("@e")
&& ref_id.len() >= 3
&& ref_id.len() <= 12
&& ref_id[2..].chars().all(|c| c.is_ascii_digit())
&& ref_id[2..].parse::<u32>().is_ok_and(|n| n > 0);
if valid {
return Ok(());
}
Err(AppError::invalid_input(format!(
"Invalid ref_id '{ref_id}': must match @e{{N}} where N is a positive integer"
)))
}
fn requires_accessibility(permission: PermissionNeed) -> bool {
matches!(
permission,

4
src/command_policy_tests.rs
View file

@ -4,6 +4,8 @@ use crate::cli_args::{RefArgs, ScreenshotArgs, SnapshotArgs};
use agent_desktop_core::{PermissionReport, PermissionState};
use clap::CommandFactory;
const VALID_REF_ID: &str = "@e1";
#[test]
fn every_cli_subcommand_has_policy() {
for subcommand in Cli::command().get_subcommands() {
@ -117,7 +119,7 @@ fn accessibility_denial_is_preflighted_for_ax_commands() {
automation: PermissionState::NotRequired,
};
let command = Commands::Click(crate::cli_args::RefArgs {
ref_id: "@e1".into(),
ref_id: VALID_REF_ID.into(),
snapshot_id: None,
});