mirror of
https://github.com/0xrsydn/idx-cli.git
synced 2026-11-07 09:45:36 +00:00
fix: address final review (path traversal, sync races, MCP limits)
Findings from a Codex (gpt-6.1-sol, xhigh) review of v0.2.4..HEAD, each verified in the code before fixing: - high: ticker symbols reached cache file paths unchecked, so a symbol like `../../REPORT` read (and deleted, if unparseable) .json files outside the cache, reachable through MCP get_quotes. resolve_symbol now accepts only ticker characters, and cache keys are encoded as single safe path components. - high: sync decided before downloading and replaced the database without rechecking; an import that landed meanwhile was erased. It now rechecks the local state before installing, and idx-mcp runs ownership_sync exclusively with ownership reads. - replaces_sqlite_sha256 never downgrades history: the client requires the rebuild to keep at least the local release count, and the publisher lists a replaced build only then. - cache writes are atomic (temp file + rename), so concurrent readers never see, and delete, a partial entry. - idx-mcp caps symbols (20, deduplicated) and limits, and runs at most 4 tool calls at once. - each Yahoo client gets its own cookie jar, paired with its crumb. - ZIP entries (XLSX, KSEI archives) are read with a 256 MiB bound; non-ASCII archive dates and huge percentages are parse errors instead of panics or overflow. - agent-smoke --keep deletes every copied agent config, not only auth files. CLI goldens unchanged; the MCP tool golden changes only to state the new caps. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
5a3ce462cc
commit
60634d3aef
16 changed files with 310 additions and 59 deletions
|
|
@ -163,6 +163,19 @@ pub fn resolve_symbol(symbol: &str, exchange: &str) -> Result<String, IdxError>
|
|||
"ticker symbol cannot be empty".into(),
|
||||
));
|
||||
}
|
||||
// Tickers name cache files and provider URLs: letters, digits, and the
|
||||
// separators real symbols use (BBCA.JK, BRK-B, ^JKSE), nothing else.
|
||||
if trimmed.len() > 32
|
||||
|| trimmed.contains("..")
|
||||
|| !trimmed
|
||||
.chars()
|
||||
.all(|ch| ch.is_ascii_alphanumeric() || matches!(ch, '.' | '-' | '^' | '='))
|
||||
{
|
||||
return Err(IdxError::InvalidInput(format!(
|
||||
"invalid ticker symbol '{}'",
|
||||
symbol.trim()
|
||||
)));
|
||||
}
|
||||
if let Some((base, suffix)) = trimmed.rsplit_once('.')
|
||||
&& !base.is_empty()
|
||||
&& !suffix.is_empty()
|
||||
|
|
@ -301,6 +314,26 @@ mod tests {
|
|||
assert_eq!(resolve_symbol("BBCA", "JK").unwrap(), "BBCA.JK");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_symbols_that_are_not_tickers() {
|
||||
for bad in [
|
||||
"../../REPORT",
|
||||
"a/b",
|
||||
"a\\b",
|
||||
"BB CA",
|
||||
"BBCA..JK",
|
||||
"BBCA\u{0}",
|
||||
&"A".repeat(40),
|
||||
] {
|
||||
assert!(
|
||||
matches!(resolve_symbol(bad, "JK"), Err(IdxError::InvalidInput(_))),
|
||||
"{bad:?} accepted"
|
||||
);
|
||||
}
|
||||
assert_eq!(resolve_symbol("^jkse", "JK").unwrap(), "^JKSE.JK");
|
||||
assert_eq!(resolve_symbol("BRK-B.US", "JK").unwrap(), "BRK-B.US");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn selected_provider_exposes_expected_capabilities_for_msn() {
|
||||
let provider = build_selected_provider(ProviderKind::Msn, false, false);
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ use std::path::{Path, PathBuf};
|
|||
use std::process::Command;
|
||||
use std::process::Stdio;
|
||||
use std::sync::Mutex;
|
||||
use std::sync::atomic::{AtomicU64, Ordering};
|
||||
use std::time::Duration;
|
||||
|
||||
use crate::api::types::{Interval, Period};
|
||||
|
|
@ -19,6 +20,9 @@ const CRUMB_FETCH_URL: &str = "https://query1.finance.yahoo.com/v1/test/getcrumb
|
|||
pub(super) struct YahooClient {
|
||||
agent: ureq::Agent,
|
||||
crumb: Mutex<Option<String>>,
|
||||
/// One jar per client, paired with its crumb: clients in the same process
|
||||
/// (concurrent MCP calls) must not overwrite each other's cookies.
|
||||
cookie_jar: PathBuf,
|
||||
}
|
||||
|
||||
impl YahooClient {
|
||||
|
|
@ -29,9 +33,16 @@ impl YahooClient {
|
|||
.build()
|
||||
.into();
|
||||
|
||||
static NEXT_JAR: AtomicU64 = AtomicU64::new(0);
|
||||
let cookie_jar = std::env::temp_dir().join(format!(
|
||||
"idx_yf_{}_{}.txt",
|
||||
std::process::id(),
|
||||
NEXT_JAR.fetch_add(1, Ordering::Relaxed)
|
||||
));
|
||||
Self {
|
||||
agent,
|
||||
crumb: Mutex::new(None),
|
||||
cookie_jar,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -49,10 +60,6 @@ impl YahooClient {
|
|||
)
|
||||
}
|
||||
|
||||
fn cookie_jar_path() -> PathBuf {
|
||||
std::env::temp_dir().join(format!("idx_yf_{}.txt", std::process::id()))
|
||||
}
|
||||
|
||||
pub(super) fn parse_crumb_body(raw: &str) -> Result<String, IdxError> {
|
||||
let crumb = raw.trim();
|
||||
if crumb.is_empty() {
|
||||
|
|
@ -123,7 +130,7 @@ impl YahooClient {
|
|||
fn fetch_crumb_via_curl(&self) -> Result<String, IdxError> {
|
||||
let binary = curl_impersonate::chrome_curl_binary()?;
|
||||
|
||||
let cookie_jar = Self::cookie_jar_path();
|
||||
let cookie_jar = &self.cookie_jar;
|
||||
let cookie_jar_str = cookie_jar.to_str().ok_or_else(|| {
|
||||
IdxError::Io(format!(
|
||||
"failed to encode Yahoo cookie jar path {}",
|
||||
|
|
@ -235,7 +242,7 @@ impl YahooClient {
|
|||
) -> Result<QuoteSummaryResponse, IdxError> {
|
||||
for auth_attempt in 0..2 {
|
||||
let crumb = self.get_or_init_crumb()?;
|
||||
let cookie_header = match Self::cookie_header_from_jar(&Self::cookie_jar_path()) {
|
||||
let cookie_header = match Self::cookie_header_from_jar(&self.cookie_jar) {
|
||||
Ok(header) => header,
|
||||
Err(err) => {
|
||||
runtime::warn(format!("failed to parse Yahoo cookie jar: {err}"));
|
||||
|
|
@ -284,6 +291,12 @@ fn jitter() -> Duration {
|
|||
Duration::from_millis(fastrand::u64(0..100))
|
||||
}
|
||||
|
||||
impl Drop for YahooClient {
|
||||
fn drop(&mut self) {
|
||||
let _ = std::fs::remove_file(&self.cookie_jar);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::YahooClient;
|
||||
|
|
|
|||
|
|
@ -136,7 +136,14 @@ impl Cache {
|
|||
};
|
||||
let raw = serde_json::to_string_pretty(&entry)
|
||||
.map_err(|e| IdxError::ParseError(e.to_string()))?;
|
||||
fs::write(path, raw).map_err(|e| IdxError::Io(e.to_string()))
|
||||
// Write then rename, so a concurrent reader never sees a partial
|
||||
// entry (and deletes it as corrupt).
|
||||
let temp = path.with_extension(format!("json.{}.tmp", fastrand::u64(..)));
|
||||
fs::write(&temp, raw).map_err(|e| IdxError::Io(e.to_string()))?;
|
||||
fs::rename(&temp, &path).map_err(|e| {
|
||||
let _ = fs::remove_file(&temp);
|
||||
IdxError::Io(e.to_string())
|
||||
})
|
||||
}
|
||||
|
||||
pub fn info(&self) -> Result<CacheInfo, IdxError> {
|
||||
|
|
@ -244,10 +251,30 @@ impl Cache {
|
|||
}
|
||||
|
||||
fn entry_path(&self, data_type: &str, symbol: &str) -> PathBuf {
|
||||
self.root.join(data_type).join(format!("{symbol}.json"))
|
||||
self.root
|
||||
.join(safe_file_name(data_type))
|
||||
.join(format!("{}.json", safe_file_name(symbol)))
|
||||
}
|
||||
}
|
||||
|
||||
/// Keys become single path components: anything that could separate or
|
||||
/// climb directories is percent-encoded. Keys made of letters, digits, and
|
||||
/// `._-:^=` (every key idx produces) are unchanged.
|
||||
fn safe_file_name(key: &str) -> String {
|
||||
let mut out = String::with_capacity(key.len());
|
||||
for (i, byte) in key.bytes().enumerate() {
|
||||
let safe = byte.is_ascii_alphanumeric()
|
||||
|| matches!(byte, b'_' | b'-' | b':' | b'^' | b'=')
|
||||
|| (byte == b'.' && i > 0);
|
||||
if safe {
|
||||
out.push(byte as char);
|
||||
} else {
|
||||
out.push_str(&format!("%{byte:02X}"));
|
||||
}
|
||||
}
|
||||
out
|
||||
}
|
||||
|
||||
fn is_expired(fetched_at: DateTime<Utc>, ttl_secs: u64) -> bool {
|
||||
let ttl =
|
||||
chrono::Duration::from_std(Duration::from_secs(ttl_secs)).unwrap_or(chrono::Duration::MAX);
|
||||
|
|
@ -275,7 +302,7 @@ mod tests {
|
|||
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
use super::{Cache, CacheEntry};
|
||||
use super::{Cache, CacheEntry, safe_file_name};
|
||||
|
||||
static TMP_COUNTER: AtomicUsize = AtomicUsize::new(0);
|
||||
|
||||
|
|
@ -377,4 +404,22 @@ mod tests {
|
|||
// The corrupted file should be deleted
|
||||
assert!(!path.exists(), "corrupted file should be deleted");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cache_keys_cannot_leave_the_cache_root() {
|
||||
assert_eq!(safe_file_name("BBCA.JK-3mo-1d"), "BBCA.JK-3mo-1d");
|
||||
assert_eq!(
|
||||
safe_file_name("top-performers:id:50"),
|
||||
"top-performers:id:50"
|
||||
);
|
||||
assert_eq!(safe_file_name("../../REPORT"), "%2E.%2F..%2FREPORT");
|
||||
assert_eq!(safe_file_name(".."), "%2E.");
|
||||
|
||||
let cache = Cache::at(std::path::PathBuf::from("/cache/idx"));
|
||||
let path = cache.entry_path("msn-quote", "../../REPORT");
|
||||
assert_eq!(
|
||||
path.parent().unwrap(),
|
||||
std::path::Path::new("/cache/idx/msn-quote")
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
use std::fs;
|
||||
use std::io::{Cursor, Read};
|
||||
use std::io::Cursor;
|
||||
use std::path::Path;
|
||||
|
||||
use chrono::NaiveDate;
|
||||
|
|
@ -177,11 +177,7 @@ fn extract_txt_from_zip(path: &Path) -> Result<String, IdxError> {
|
|||
continue;
|
||||
}
|
||||
|
||||
let mut output = String::new();
|
||||
file.read_to_string(&mut output).map_err(|e| {
|
||||
IdxError::ParseError(format!("failed to decode KSEI archive TXT entry: {e}"))
|
||||
})?;
|
||||
return Ok(output);
|
||||
return super::read_zip_entry_bounded(&mut file, "KSEI archive TXT entry");
|
||||
}
|
||||
|
||||
Err(IdxError::ParseError(
|
||||
|
|
@ -201,7 +197,8 @@ fn validate_header(header: &str) -> Result<(), IdxError> {
|
|||
|
||||
fn parse_archive_date(raw: &str) -> Result<NaiveDate, IdxError> {
|
||||
let trimmed = raw.trim();
|
||||
if trimmed.len() != 11 {
|
||||
// Fixed byte offsets below; non-ASCII input would split a character.
|
||||
if trimmed.len() != 11 || !trimmed.is_ascii() {
|
||||
return Err(IdxError::ParseError(format!(
|
||||
"invalid KSEI archive date `{trimmed}`"
|
||||
)));
|
||||
|
|
@ -357,4 +354,14 @@ mod tests {
|
|||
assert!(pdf_local_cp.total_shares <= archive_local_cp.total_shares);
|
||||
assert!(pdf_local_cp.percentage_bps <= archive_local_cp.percentage_bps);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn non_ascii_archive_dates_are_parse_errors_not_panics() {
|
||||
// 11 bytes, but byte offsets would split the 'é'.
|
||||
assert!(super::parse_archive_date("1éJan-2026").is_err());
|
||||
assert_eq!(
|
||||
super::parse_archive_date("27-FEB-2026").unwrap(),
|
||||
chrono::NaiveDate::from_ymd_opt(2026, 2, 27).unwrap()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -81,7 +81,10 @@ pub fn parse_id_percentage(s: &str) -> Result<i64, IdxError> {
|
|||
IdxError::ParseError(format!("failed to parse Indonesian percentage '{s}': {e}"))
|
||||
})?;
|
||||
|
||||
Ok((whole_i * 100) + frac_i)
|
||||
whole_i
|
||||
.checked_mul(100)
|
||||
.and_then(|bps| bps.checked_add(frac_i))
|
||||
.ok_or_else(|| IdxError::ParseError(format!("Indonesian percentage '{s}' is out of range")))
|
||||
}
|
||||
|
||||
/// Parse KSEI date format to NaiveDate.
|
||||
|
|
@ -453,4 +456,10 @@ mod tests {
|
|||
|
||||
assert!(normalize_ksei_row(&raw).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn huge_percentages_are_parse_errors_not_overflow() {
|
||||
assert!(super::parse_id_percentage("9223372036854775807,99").is_err());
|
||||
assert_eq!(super::parse_id_percentage("54,94").unwrap(), 5494);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -8,3 +8,27 @@ pub mod search;
|
|||
pub mod snapshot;
|
||||
pub mod types;
|
||||
pub mod xlsx;
|
||||
|
||||
use std::io::Read;
|
||||
|
||||
use crate::error::IdxError;
|
||||
|
||||
/// Largest decompressed ZIP entry read into memory. Real IDX worksheets and
|
||||
/// KSEI TXT archives are tens of MB; this bounds a hostile "zip bomb".
|
||||
const MAX_ZIP_ENTRY_BYTES: u64 = 256 * 1024 * 1024;
|
||||
|
||||
/// Reads one decompressed ZIP entry as UTF-8, failing past the size limit.
|
||||
pub(crate) fn read_zip_entry_bounded(entry: impl Read, what: &str) -> Result<String, IdxError> {
|
||||
let mut bytes = Vec::new();
|
||||
entry
|
||||
.take(MAX_ZIP_ENTRY_BYTES + 1)
|
||||
.read_to_end(&mut bytes)
|
||||
.map_err(|e| IdxError::ParseError(format!("failed to read {what}: {e}")))?;
|
||||
if bytes.len() as u64 > MAX_ZIP_ENTRY_BYTES {
|
||||
return Err(IdxError::ParseError(format!(
|
||||
"{what} expands beyond {MAX_ZIP_ENTRY_BYTES} bytes"
|
||||
)));
|
||||
}
|
||||
String::from_utf8(bytes)
|
||||
.map_err(|e| IdxError::ParseError(format!("failed to decode {what}: {e}")))
|
||||
}
|
||||
|
|
|
|||
|
|
@ -202,6 +202,14 @@ pub fn sync_snapshot(
|
|||
|
||||
let install_result = (|| -> Result<(), IdxError> {
|
||||
validate_snapshot_db(&temp_path, &manifest.snapshot)?;
|
||||
// The decision was made before the download; an import or edit that
|
||||
// landed since must not be overwritten without --force.
|
||||
if !force && !same_local_state(local_state.as_ref(), inspect_local_db(db_path)?.as_ref()) {
|
||||
return Err(IdxError::DatabaseError(
|
||||
"the local ownership database changed during sync; rerun `idx ownership sync`"
|
||||
.to_string(),
|
||||
));
|
||||
}
|
||||
install_snapshot_file(&temp_path, db_path)
|
||||
})();
|
||||
|
||||
|
|
@ -362,6 +370,21 @@ fn inspect_local_db(db_path: &Path) -> Result<Option<LocalSnapshotState>, IdxErr
|
|||
}))
|
||||
}
|
||||
|
||||
fn same_local_state(
|
||||
before: Option<&LocalSnapshotState>,
|
||||
after: Option<&LocalSnapshotState>,
|
||||
) -> bool {
|
||||
match (before, after) {
|
||||
(None, None) => true,
|
||||
(Some(before), Some(after)) => {
|
||||
before.file_sha256.is_some()
|
||||
&& before.file_sha256 == after.file_sha256
|
||||
&& before.release_count == after.release_count
|
||||
}
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
/// Moves committed WAL frames into the database file, so its hash covers local
|
||||
/// edits too. Returns false when changes remain in the WAL.
|
||||
fn checkpoint_wal(db_path: &Path) -> bool {
|
||||
|
|
@ -446,7 +469,9 @@ fn build_sync_decision(
|
|||
};
|
||||
}
|
||||
|
||||
if snapshot
|
||||
// A rebuild never silently drops history the local database has.
|
||||
if snapshot.release_count >= local.release_count
|
||||
&& snapshot
|
||||
.replaces_sqlite_sha256
|
||||
.iter()
|
||||
.any(|sha256| file_sha256.is_some_and(|local| sha256.eq_ignore_ascii_case(local)))
|
||||
|
|
@ -909,6 +934,20 @@ mod tests {
|
|||
assert!(!decision.should_download);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_sync_decision_never_drops_history_through_a_rebuild() {
|
||||
// A forced same-date rebuild with fewer releases lists the old build;
|
||||
// a client holding it keeps its history instead of downgrading.
|
||||
let earlier = "e".repeat(64);
|
||||
let mut snapshot = same_date_snapshot(vec![earlier.clone()]);
|
||||
snapshot.release_count = 1;
|
||||
let mut local = same_release_local(&snapshot, &earlier);
|
||||
local.release_count = 3;
|
||||
|
||||
let decision = build_sync_decision(Some(&local), &snapshot, false);
|
||||
assert!(!decision.should_download, "{}", decision.reason);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_sync_decision_keeps_a_locally_modified_database() {
|
||||
// Same release metadata but a file matching no published build (local
|
||||
|
|
|
|||
|
|
@ -9,7 +9,7 @@
|
|||
|
||||
use std::collections::HashMap;
|
||||
use std::fs;
|
||||
use std::io::{Cursor, Read};
|
||||
use std::io::Cursor;
|
||||
use std::path::Path;
|
||||
|
||||
use chrono::{Duration, NaiveDate};
|
||||
|
|
@ -83,11 +83,7 @@ fn read_zip_entry(
|
|||
)));
|
||||
}
|
||||
};
|
||||
let mut output = String::new();
|
||||
file.read_to_string(&mut output).map_err(|e| {
|
||||
IdxError::ParseError(format!("failed to decode ownership XLSX entry {name}: {e}"))
|
||||
})?;
|
||||
Ok(Some(output))
|
||||
super::read_zip_entry_bounded(&mut file, &format!("ownership XLSX entry {name}")).map(Some)
|
||||
}
|
||||
|
||||
fn first_worksheet_name(zip: &mut zip::ZipArchive<Cursor<&[u8]>>) -> Result<String, IdxError> {
|
||||
|
|
|
|||
|
|
@ -24,7 +24,7 @@ rmcp = { version = "3.5", default-features = false, features = ["server", "macro
|
|||
schemars = "1"
|
||||
serde.workspace = true
|
||||
serde_json.workspace = true
|
||||
tokio = { version = "1", features = ["macros", "rt-multi-thread", "io-std"] }
|
||||
tokio = { version = "1", features = ["macros", "rt-multi-thread", "io-std", "sync"] }
|
||||
|
||||
[dev-dependencies]
|
||||
# Tests drive the binary with the mock provider; release builds exclude it.
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@ use serde::Deserialize;
|
|||
|
||||
#[derive(Debug, Deserialize, JsonSchema)]
|
||||
pub struct SymbolsParams {
|
||||
/// IDX tickers, e.g. ["BBCA", "BBRI"]; the `.JK` suffix is optional.
|
||||
/// IDX tickers, e.g. ["BBCA", "BBRI"]; the `.JK` suffix is optional. At most 20.
|
||||
pub symbols: Vec<String>,
|
||||
}
|
||||
|
||||
|
|
@ -36,7 +36,7 @@ pub struct ScreenParams {
|
|||
#[serde(default)]
|
||||
#[schemars(schema_with = "region_schema")]
|
||||
pub region: Option<String>,
|
||||
/// Maximum results (default 50).
|
||||
/// Maximum results (default 50, at most 200).
|
||||
#[serde(default)]
|
||||
pub limit: Option<usize>,
|
||||
}
|
||||
|
|
@ -130,7 +130,7 @@ pub struct EntityParams {
|
|||
pub struct SearchParams {
|
||||
/// Full-text query over investor names.
|
||||
pub query: String,
|
||||
/// Maximum results (default 20).
|
||||
/// Maximum results (default 20, at most 100).
|
||||
#[serde(default)]
|
||||
pub limit: Option<usize>,
|
||||
}
|
||||
|
|
@ -144,7 +144,7 @@ pub struct ChangesParams {
|
|||
/// Only this ticker's changes.
|
||||
#[serde(default)]
|
||||
pub ticker: Option<String>,
|
||||
/// Maximum rows returned (default 200); `total` reports the full count.
|
||||
/// Maximum rows returned (default 200, at most 2000); `total` reports the full count.
|
||||
#[serde(default)]
|
||||
pub limit: Option<usize>,
|
||||
}
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@ use rmcp::model::{CallToolResult, Implementation, ServerCapabilities, ServerConf
|
|||
use rmcp::{ErrorData, ServerHandler, tool, tool_handler, tool_router};
|
||||
use serde::Serialize;
|
||||
use serde_json::{Map, Value, json};
|
||||
use tokio::sync::{RwLock, Semaphore};
|
||||
|
||||
use crate::params::{
|
||||
AnalysisKind, AnalyzeParams, ChangesParams, CompanyParams, CompanySection, EntityParams,
|
||||
|
|
@ -26,6 +27,18 @@ const NEWS_LIMIT: usize = 10;
|
|||
const SCREEN_LIMIT: usize = 50;
|
||||
const SEARCH_LIMIT: usize = 20;
|
||||
const CHANGES_LIMIT: usize = 200;
|
||||
// Hard caps on what one call may request, so a misbehaving or prompt-injected
|
||||
// agent cannot turn a call into unbounded upstream traffic or output.
|
||||
const MAX_SYMBOLS: usize = 20;
|
||||
const MAX_SCREEN_LIMIT: usize = 200;
|
||||
const MAX_SEARCH_LIMIT: usize = 100;
|
||||
const MAX_CHANGES_LIMIT: usize = 2000;
|
||||
|
||||
/// Tool calls running at once; the rest wait their turn.
|
||||
static CALL_SLOTS: Semaphore = Semaphore::const_new(4);
|
||||
/// `ownership_sync` replaces the database file: it runs alone, while
|
||||
/// ownership reads may run together.
|
||||
static OWNERSHIP: RwLock<()> = RwLock::const_new(());
|
||||
|
||||
const INSTRUCTIONS: &str = "Indonesian stock market (IDX) data. Market and financial \
|
||||
tools fetch live data (cached on disk) and wrap it as {data, source, fetched_at, stale}; \
|
||||
|
|
@ -79,6 +92,7 @@ impl IdxServer {
|
|||
} else {
|
||||
"run `idx ownership sync`, or restart idx-mcp with --allow-sync"
|
||||
};
|
||||
let _read = OWNERSHIP.read().await;
|
||||
blocking(move || {
|
||||
if !investor.status()?.populated {
|
||||
return Err(IdxError::NoMarketData(format!(
|
||||
|
|
@ -98,10 +112,10 @@ impl IdxServer {
|
|||
for one or more IDX tickers."
|
||||
)]
|
||||
async fn get_quotes(&self, params: Parameters<SymbolsParams>) -> ToolResult {
|
||||
let SymbolsParams { symbols } = params.0;
|
||||
if let Some(err) = empty_input("symbols", &symbols) {
|
||||
return err;
|
||||
}
|
||||
let symbols = match checked_symbols(params.0.symbols) {
|
||||
Ok(symbols) => symbols,
|
||||
Err(err) => return err,
|
||||
};
|
||||
self.live(move |idx| {
|
||||
let quotes = idx.market().quotes(&symbols, CachePolicy::Default)?;
|
||||
Ok(json!({ "quotes": quotes }))
|
||||
|
|
@ -143,7 +157,7 @@ for one or more IDX tickers."
|
|||
.unwrap_or("top-performers")
|
||||
.parse::<ScreenFilter>()?,
|
||||
region: region.as_deref().unwrap_or("id").parse::<ScreenRegion>()?,
|
||||
limit: limit.unwrap_or(SCREEN_LIMIT),
|
||||
limit: bounded_limit("limit", limit, SCREEN_LIMIT, MAX_SCREEN_LIMIT)?,
|
||||
};
|
||||
idx.market().screen(&query, CachePolicy::Default)
|
||||
})
|
||||
|
|
@ -232,10 +246,10 @@ are listed under `errors`; the call fails only if every analysis fails."
|
|||
tickers. Tickers that fail are listed under `failures`."
|
||||
)]
|
||||
async fn compare_stocks(&self, params: Parameters<SymbolsParams>) -> ToolResult {
|
||||
let SymbolsParams { symbols } = params.0;
|
||||
if let Some(err) = empty_input("symbols", &symbols) {
|
||||
return err;
|
||||
}
|
||||
let symbols = match checked_symbols(params.0.symbols) {
|
||||
Ok(symbols) => symbols,
|
||||
Err(err) => return err,
|
||||
};
|
||||
self.live(move |idx| {
|
||||
let comparison = idx.analysis().compare(&symbols, CachePolicy::Default)?;
|
||||
let failures: Vec<Value> = comparison
|
||||
|
|
@ -273,7 +287,8 @@ with concentration metrics (top1, top3, HHI, free float)."
|
|||
async fn search_investors(&self, params: Parameters<SearchParams>) -> ToolResult {
|
||||
let SearchParams { query, limit } = params.0;
|
||||
self.local(move |investor| {
|
||||
let entities = investor.search(&query, limit.unwrap_or(SEARCH_LIMIT))?;
|
||||
let limit = bounded_limit("limit", limit, SEARCH_LIMIT, MAX_SEARCH_LIMIT)?;
|
||||
let entities = investor.search(&query, limit)?;
|
||||
Ok(json!({ "entities": entities }))
|
||||
})
|
||||
.await
|
||||
|
|
@ -293,7 +308,12 @@ release dates, optionally for one ticker."
|
|||
self.local(move |investor| {
|
||||
let mut changes = investor.changes(&from, &to, ticker.as_deref())?;
|
||||
let total = changes.len();
|
||||
changes.truncate(limit.unwrap_or(CHANGES_LIMIT));
|
||||
changes.truncate(bounded_limit(
|
||||
"limit",
|
||||
limit,
|
||||
CHANGES_LIMIT,
|
||||
MAX_CHANGES_LIMIT,
|
||||
)?);
|
||||
Ok(json!({ "from": from, "to": to, "total": total, "changes": changes }))
|
||||
})
|
||||
.await
|
||||
|
|
@ -305,6 +325,7 @@ KSEI release dates."
|
|||
)]
|
||||
async fn ownership_status(&self) -> ToolResult {
|
||||
let investor = Investor::new(&self.config);
|
||||
let _read = OWNERSHIP.read().await;
|
||||
blocking(move || investor.status()).await
|
||||
}
|
||||
|
||||
|
|
@ -315,6 +336,7 @@ the local database."
|
|||
async fn ownership_sync(&self, params: Parameters<SyncParams>) -> ToolResult {
|
||||
let SyncParams { force } = params.0;
|
||||
let investor = Investor::new(&self.config);
|
||||
let _write = OWNERSHIP.write().await;
|
||||
blocking(move || investor.sync(None, force)).await
|
||||
}
|
||||
}
|
||||
|
|
@ -380,6 +402,10 @@ where
|
|||
T: Serialize + Send + 'static,
|
||||
F: FnOnce() -> Result<T, IdxError> + Send + 'static,
|
||||
{
|
||||
let _slot = CALL_SLOTS
|
||||
.acquire()
|
||||
.await
|
||||
.map_err(|err| ErrorData::internal_error(err.to_string(), None))?;
|
||||
let result = tokio::task::spawn_blocking(move || f().and_then(to_json))
|
||||
.await
|
||||
.map_err(|err| ErrorData::internal_error(err.to_string(), None))?;
|
||||
|
|
@ -389,15 +415,55 @@ where
|
|||
})
|
||||
}
|
||||
|
||||
/// An explicitly empty list is an input error, as in the CLI, rather than an
|
||||
/// empty success an agent could mistake for data.
|
||||
fn empty_input<T>(field: &str, values: &[T]) -> Option<ToolResult> {
|
||||
values.is_empty().then(|| {
|
||||
let err = IdxError::InvalidInput(format!("{field} must not be empty"));
|
||||
fn input_error(message: String) -> ToolResult {
|
||||
let err = IdxError::InvalidInput(message);
|
||||
Ok(CallToolResult::structured_error(Value::Object(error_body(
|
||||
&err,
|
||||
))))
|
||||
})
|
||||
}
|
||||
|
||||
/// An explicitly empty list is an input error, as in the CLI, rather than an
|
||||
/// empty success an agent could mistake for data.
|
||||
fn empty_input<T>(field: &str, values: &[T]) -> Option<ToolResult> {
|
||||
values
|
||||
.is_empty()
|
||||
.then(|| input_error(format!("{field} must not be empty")))
|
||||
}
|
||||
|
||||
/// Non-empty, deduplicated (case-insensitively, first wins), and capped.
|
||||
fn checked_symbols(symbols: Vec<String>) -> Result<Vec<String>, ToolResult> {
|
||||
if let Some(err) = empty_input("symbols", &symbols) {
|
||||
return Err(err);
|
||||
}
|
||||
let mut seen = std::collections::HashSet::new();
|
||||
let symbols: Vec<String> = symbols
|
||||
.into_iter()
|
||||
.filter(|symbol| seen.insert(symbol.trim().to_uppercase()))
|
||||
.collect();
|
||||
if symbols.len() > MAX_SYMBOLS {
|
||||
return Err(input_error(format!(
|
||||
"at most {MAX_SYMBOLS} symbols per call (got {})",
|
||||
symbols.len()
|
||||
)));
|
||||
}
|
||||
Ok(symbols)
|
||||
}
|
||||
|
||||
fn bounded_limit(
|
||||
field: &str,
|
||||
value: Option<usize>,
|
||||
default: usize,
|
||||
max: usize,
|
||||
) -> Result<usize, IdxError> {
|
||||
match value.unwrap_or(default) {
|
||||
0 => Err(IdxError::InvalidInput(format!(
|
||||
"{field} must be at least 1"
|
||||
))),
|
||||
n if n > max => Err(IdxError::InvalidInput(format!(
|
||||
"{field} must be at most {max} (got {n})"
|
||||
))),
|
||||
n => Ok(n),
|
||||
}
|
||||
}
|
||||
|
||||
fn to_json<T: Serialize>(value: T) -> Result<Value, IdxError> {
|
||||
|
|
|
|||
|
|
@ -43,7 +43,7 @@
|
|||
"$schema": "https://json-schema.org/draft/2020-12/schema",
|
||||
"properties": {
|
||||
"symbols": {
|
||||
"description": "IDX tickers, e.g. [\"BBCA\", \"BBRI\"]; the `.JK` suffix is optional.",
|
||||
"description": "IDX tickers, e.g. [\"BBCA\", \"BBRI\"]; the `.JK` suffix is optional. At most 20.",
|
||||
"items": {
|
||||
"type": "string"
|
||||
},
|
||||
|
|
@ -125,7 +125,7 @@
|
|||
},
|
||||
"limit": {
|
||||
"default": null,
|
||||
"description": "Maximum rows returned (default 200); `total` reports the full count.",
|
||||
"description": "Maximum rows returned (default 200, at most 2000); `total` reports the full count.",
|
||||
"format": "uint",
|
||||
"minimum": 0,
|
||||
"type": [
|
||||
|
|
@ -202,7 +202,7 @@
|
|||
"$schema": "https://json-schema.org/draft/2020-12/schema",
|
||||
"properties": {
|
||||
"symbols": {
|
||||
"description": "IDX tickers, e.g. [\"BBCA\", \"BBRI\"]; the `.JK` suffix is optional.",
|
||||
"description": "IDX tickers, e.g. [\"BBCA\", \"BBRI\"]; the `.JK` suffix is optional. At most 20.",
|
||||
"items": {
|
||||
"type": "string"
|
||||
},
|
||||
|
|
@ -278,7 +278,7 @@
|
|||
},
|
||||
"limit": {
|
||||
"default": null,
|
||||
"description": "Maximum results (default 50).",
|
||||
"description": "Maximum results (default 50, at most 200).",
|
||||
"format": "uint",
|
||||
"minimum": 0,
|
||||
"type": [
|
||||
|
|
@ -310,7 +310,7 @@
|
|||
"properties": {
|
||||
"limit": {
|
||||
"default": null,
|
||||
"description": "Maximum results (default 20).",
|
||||
"description": "Maximum results (default 20, at most 100).",
|
||||
"format": "uint",
|
||||
"minimum": 0,
|
||||
"type": [
|
||||
|
|
|
|||
|
|
@ -279,6 +279,22 @@ fn errors_carry_cli_codes() {
|
|||
assert_eq!(body["code"], "INVALIDINPUT", "{tool}");
|
||||
}
|
||||
|
||||
let many: Vec<String> = (0..21).map(|i| format!("S{i}")).collect();
|
||||
for (tool, arguments) in [
|
||||
("get_quotes", json!({"symbols": many})),
|
||||
("screen_stocks", json!({"limit": 201})),
|
||||
("screen_stocks", json!({"limit": 0})),
|
||||
("get_quotes", json!({"symbols": ["../../REPORT"]})),
|
||||
] {
|
||||
let (is_error, body) = client.call(tool, arguments.clone());
|
||||
assert!(is_error, "{tool} {arguments}: {body}");
|
||||
assert_eq!(body["code"], "INVALIDINPUT", "{tool} {arguments}");
|
||||
}
|
||||
// Duplicates collapse instead of counting against the cap.
|
||||
let repeated = vec!["BBCA"; 50];
|
||||
let quotes = client.ok("get_quotes", json!({"symbols": repeated}));
|
||||
assert_eq!(quotes["quotes"].as_array().unwrap().len(), 1);
|
||||
|
||||
// Empty ownership database: investor tools point at the sync path.
|
||||
let status = client.ok("ownership_status", json!({}));
|
||||
assert_eq!(status["populated"], false);
|
||||
|
|
|
|||
|
|
@ -92,7 +92,7 @@ tests/agents/agent-smoke.sh --live --case shareholders --keep # real da
|
|||
your own agent settings and MCP servers are not loaded or changed. Logins
|
||||
are copied from the agent's home (Codex `auth.json`, pi and Hermes auth
|
||||
files) into a temp dir that is deleted afterwards; `--keep` keeps the run
|
||||
output but still deletes the copied credentials.
|
||||
output but still deletes every copied agent config.
|
||||
- Tools are auto-approved for the run (Claude `dontAsk` with
|
||||
`mcp__idx__*`, Codex `default_tools_approval_mode="approve"`); Claude and pi
|
||||
also get no built-in tools, so they can only answer through MCP.
|
||||
|
|
|
|||
|
|
@ -464,7 +464,9 @@ MANIFEST_PATH="$OUTPUT_DIR/ownership-snapshot-manifest.json"
|
|||
|
||||
# A rebuild for the published as-of date (e.g. after a parser fix) lists the
|
||||
# builds it replaces, so clients still holding one update without --force.
|
||||
if [[ "$BUILT_AS_OF" == "$PUBLISHED_AS_OF" && "$PUBLISHED_SHA256" =~ ^[0-9a-fA-F]{64}$ ]]; then
|
||||
# Only when it keeps at least the published history: clients never downgrade.
|
||||
if [[ "$BUILT_AS_OF" == "$PUBLISHED_AS_OF" && "$PUBLISHED_SHA256" =~ ^[0-9a-fA-F]{64}$ ]] \
|
||||
&& [[ "$PUBLISHED_RELEASE_COUNT" =~ ^[0-9]+$ ]] && (( BUILT_RELEASE_COUNT >= PUBLISHED_RELEASE_COUNT )); then
|
||||
replaces_tmp="$PUBLISH_WORKDIR/manifest-replaces.json"
|
||||
jq --arg published "$PUBLISHED_SHA256" \
|
||||
--argjson previous "$(jq -c '.snapshot.replaces_sqlite_sha256 // []' <<< "$PUBLISHED_MANIFEST" 2>/dev/null || echo '[]')" '
|
||||
|
|
|
|||
|
|
@ -81,9 +81,10 @@ done
|
|||
work="$(mktemp -d "${TMPDIR:-/tmp}/idx-agent-smoke.XXXXXX")"
|
||||
cleanup() {
|
||||
if [[ "$keep" == 1 ]]; then
|
||||
# Never leave copied agent credentials behind.
|
||||
rm -f -- "$work/codex/auth.json" "$work/pi/auth.json" "$work/hermes/auth.json" "$work/hermes/.env"
|
||||
echo "agent-smoke: kept $work (copied credentials removed)"
|
||||
# Never leave copied agent logins or settings (which may hold API keys)
|
||||
# behind; each run's raw output is already under runs/.
|
||||
rm -rf -- "$work/codex" "$work/pi" "$work/hermes" "$work/opencode"
|
||||
echo "agent-smoke: kept $work (copied agent configs removed)"
|
||||
elif [[ "$work" == */idx-agent-smoke.* ]]; then
|
||||
rm -rf -- "$work"
|
||||
fi
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue