mirror of
https://github.com/lahfir/agent-desktop.git
synced 2026-08-19 21:49:23 +00:00
fix: redact --env values from trace-reachable errors; dedupe scroll_into_view
The --env KEY=VALUE parser interpolated the raw pair (including the secret value) into INVALID_ARGS messages, which reach the unredacted `message` field of command.end trace events (redaction is a field-name allowlist, not a content scanner). Report the entry index and rule only, never the value, per docs/solutions/conventions/keep-raw-arguments-out-of-trace-reachable-error-messages.md, with a regression test asserting a secret marker never appears in the message. scroll_into_view reused the hand-rolled AXScrollToVisible CFString+perform sequence that ax_helpers::try_ax_action already provides.
This commit is contained in:
parent
ec47a10b2f
commit
f329d5840f
3 changed files with 32 additions and 10 deletions
|
|
@ -1,14 +1,11 @@
|
|||
#[cfg(target_os = "macos")]
|
||||
mod imp {
|
||||
use crate::actions::ax_helpers;
|
||||
use crate::tree::AXElement;
|
||||
use accessibility_sys::{AXUIElementPerformAction, kAXErrorSuccess};
|
||||
use agent_desktop_core::error::AdapterError;
|
||||
use core_foundation::{base::TCFType, string::CFString};
|
||||
|
||||
pub fn scroll_into_view_impl(el: &AXElement) -> Result<(), AdapterError> {
|
||||
let action = CFString::new("AXScrollToVisible");
|
||||
let err = unsafe { AXUIElementPerformAction(el.0, action.as_concrete_TypeRef()) };
|
||||
if err == kAXErrorSuccess {
|
||||
if ax_helpers::try_ax_action(el, "AXScrollToVisible") {
|
||||
return Ok(());
|
||||
}
|
||||
Err(AdapterError::not_supported(
|
||||
|
|
|
|||
|
|
@ -117,8 +117,8 @@ pub(crate) fn build_launch_options(
|
|||
no_attach: bool,
|
||||
) -> Result<LaunchOptions, AppError> {
|
||||
let mut env_map = HashMap::new();
|
||||
for pair in env {
|
||||
let (key, value) = parse_env_pair(pair)?;
|
||||
for (idx, pair) in env.iter().enumerate() {
|
||||
let (key, value) = parse_env_pair(pair, idx)?;
|
||||
env_map.insert(key, value);
|
||||
}
|
||||
Ok(LaunchOptions {
|
||||
|
|
@ -129,13 +129,13 @@ pub(crate) fn build_launch_options(
|
|||
})
|
||||
}
|
||||
|
||||
fn parse_env_pair(pair: &str) -> Result<(String, String), AppError> {
|
||||
fn parse_env_pair(pair: &str, idx: usize) -> Result<(String, String), AppError> {
|
||||
let (key, value) = pair.split_once('=').ok_or_else(|| {
|
||||
AppError::invalid_input(format!("Invalid --env value '{pair}'. Expected KEY=VALUE"))
|
||||
AppError::invalid_input(format!("Invalid --env entry #{idx}: expected KEY=VALUE"))
|
||||
})?;
|
||||
if key.is_empty() {
|
||||
return Err(AppError::invalid_input(format!(
|
||||
"Invalid --env value '{pair}'. KEY must not be empty"
|
||||
"Invalid --env entry #{idx}: KEY must not be empty"
|
||||
)));
|
||||
}
|
||||
Ok((key.to_string(), value.to_string()))
|
||||
|
|
|
|||
|
|
@ -8,6 +8,31 @@ fn rejects_unknown_direction() {
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn env_parse_errors_never_echo_the_value() {
|
||||
let secret = "sk_live_supersecret_token_value";
|
||||
let no_equals = parse_env_pair(secret, 0).unwrap_err();
|
||||
let no_equals_msg = no_equals.to_string();
|
||||
assert_eq!(no_equals.code(), "INVALID_ARGS");
|
||||
assert!(
|
||||
!no_equals_msg.contains(secret),
|
||||
"malformed --env message leaked the raw value: {no_equals_msg}"
|
||||
);
|
||||
|
||||
let empty_key = format!("={secret}");
|
||||
let err = parse_env_pair(&empty_key, 3).unwrap_err();
|
||||
let err_msg = err.to_string();
|
||||
assert_eq!(err.code(), "INVALID_ARGS");
|
||||
assert!(
|
||||
!err_msg.contains(secret),
|
||||
"empty-key --env message leaked the raw value: {err_msg}"
|
||||
);
|
||||
assert!(
|
||||
err_msg.contains("#3"),
|
||||
"message should carry the entry index"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_unknown_get_property() {
|
||||
match parse_get_property("placeholder") {
|
||||
|
|
|
|||
Loading…
Reference in a new issue