fix(ownership): bound snapshot download by manifest size; clean refused builds

Addresses review on #31:
- the snapshot download limit is the manifest's size_bytes (+1, since
  ureq fails once a body reaches its limit), capped at 1 GiB; a body
  larger than advertised fails during download. New CLI test; the
  >10 MiB test also caught the off-by-one.
- a refused (oversized) standalone build removes its sqlite and
  manifest from --output-dir.
- docs: downloads are buffered, not streamed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017a5Cv5Z1NzdYKgKnTiAF4E
This commit is contained in:
hermes 2026-09-25 22:31:40 +00:00
commit fea1a0e906
4 changed files with 71 additions and 7 deletions

View file

@ -118,7 +118,8 @@ bytes) and only accept uncompressed SQLite. A larger snapshot makes
The builder therefore refuses snapshots above `--max-snapshot-bytes`
(default 10485760). The run ends `RESULT: FAILED stage=build-snapshot` and the
published snapshot stays unchanged. Newer clients stream downloads up to 1 GiB;
published snapshot stays unchanged. Newer clients accept snapshots up to the
manifest's `size_bytes` (at most 1 GiB, buffered in memory);
raise `--max-snapshot-bytes` and `--history` only after clients without the
10 MiB cap are widespread.

View file

@ -278,6 +278,13 @@ fi
if (( SNAPSHOT_BYTES > MAX_SNAPSHOT_BYTES )); then
printf 'snapshot is %s bytes (%s monthly releases), above --max-snapshot-bytes %s; lower --history\n' \
"$SNAPSHOT_BYTES" "$(jq -r '.snapshot.release_count' "$MANIFEST_PATH")" "$MAX_SNAPSHOT_BYTES" >&2
# Remove the refused pair so a standalone run leaves nothing publishable.
refused_sqlite="$(jq -r '.snapshot.download_url // empty' "$MANIFEST_PATH")"
refused_sqlite="${refused_sqlite##*/}"
if [[ -n "$refused_sqlite" && "$refused_sqlite" != */* ]]; then
rm -f "$OUTPUT_DIR/$refused_sqlite"
fi
rm -f "$MANIFEST_PATH"
exit 1
fi

View file

@ -17,8 +17,9 @@ pub const SNAPSHOT_MANIFEST_SCHEMA_VERSION: u32 = 1;
pub const DEFAULT_SNAPSHOT_MANIFEST_URL: &str = "https://github.com/0xrsydn/idx-cli/releases/download/ownership-snapshot-current/ownership-snapshot-manifest.json";
/// ureq caps response bodies at 10 MiB by default. Snapshots with several
/// months of history exceed that (history 3 is ~10.5 MB), and the download is
/// verified against the manifest's size and SHA-256 anyway, so allow up to 1 GiB.
/// months of history exceed that (history 3 is ~10.5 MB), so the download is
/// bounded by the manifest's `size_bytes` instead, never above 1 GiB. The body
/// is still verified against that size and the SHA-256 afterwards.
pub(crate) const SNAPSHOT_DOWNLOAD_LIMIT_BYTES: u64 = 1024 * 1024 * 1024;
const USER_AGENT: &str = "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/122.0.0.0 Safari/537.36";
@ -154,7 +155,20 @@ pub fn sync_snapshot(
});
}
let bytes = read_bytes(&manifest.snapshot.download_url, "ownership snapshot SQLite")?;
if manifest.snapshot.size_bytes > SNAPSHOT_DOWNLOAD_LIMIT_BYTES {
return Err(IdxError::InvalidInput(format!(
"ownership snapshot manifest reports {} bytes, above the {} byte limit",
manifest.snapshot.size_bytes, SNAPSHOT_DOWNLOAD_LIMIT_BYTES
)));
}
// A body larger than the manifest says is rejected while downloading
// (ureq fails once a body reaches its limit, hence the +1); the exact size
// and SHA-256 are verified afterwards.
let bytes = read_bytes(
&manifest.snapshot.download_url,
"ownership snapshot SQLite",
manifest.snapshot.size_bytes + 1,
)?;
validate_downloaded_bytes(&bytes, &manifest.snapshot)?;
let temp_path = build_temp_path(db_path);
@ -649,7 +663,7 @@ fn read_text(source: &str, context: &str) -> Result<String, IdxError> {
.map_err(|e| IdxError::Io(format!("failed to read {context} {}: {e}", path.display())))
}
fn read_bytes(source: &str, context: &str) -> Result<Vec<u8>, IdxError> {
fn read_bytes(source: &str, context: &str, limit_bytes: u64) -> Result<Vec<u8>, IdxError> {
if is_http_source(source) {
let response = ureq::get(source)
.header("User-Agent", USER_AGENT)
@ -662,9 +676,13 @@ fn read_bytes(source: &str, context: &str) -> Result<Vec<u8>, IdxError> {
return response
.into_body()
.with_config()
.limit(SNAPSHOT_DOWNLOAD_LIMIT_BYTES)
.limit(limit_bytes)
.read_to_vec()
.map_err(|e| IdxError::Http(format!("failed reading {context} body: {e}")));
.map_err(|e| {
IdxError::Http(format!(
"failed reading {context} body (larger than the manifest's size_bytes?): {e}"
))
});
}
let path = local_source_path(source);

View file

@ -1917,6 +1917,44 @@ fn ownership_sync_downloads_snapshot_larger_than_ten_mib_over_http() {
.stdout(predicate::str::contains("Installed ownership snapshot"));
}
#[test]
fn ownership_sync_rejects_body_larger_than_manifest_size() {
let publisher_root = test_env_dir("ownership-sync-oversized-body");
let (source_db, manifest_path) = prepare_snapshot_fixture(&publisher_root);
write_snapshot_manifest(&manifest_path, &source_db, None);
let mut body = fs::read(&source_db).expect("read snapshot");
body.extend_from_slice(&[0u8; 4096]);
let snapshot_url = spawn_single_response_server("application/octet-stream", body);
let mut manifest: Value =
serde_json::from_str(&fs::read_to_string(&manifest_path).expect("read manifest"))
.expect("parse manifest");
manifest["snapshot"]["download_url"] = Value::String(format!("{snapshot_url}/snapshot.sqlite"));
fs::write(&manifest_path, manifest.to_string()).expect("write http manifest");
let sync_root = test_env_dir("ownership-sync-oversized-body-consumer");
bin_with_root(&sync_root)
.args([
"config",
"set",
"ownership.db_path",
sync_root.join("ownership.db").to_str().unwrap(),
])
.assert()
.success();
bin_with_root(&sync_root)
.args([
"ownership",
"sync",
"--manifest",
manifest_path.to_str().unwrap(),
])
.assert()
.failure()
.stderr(predicate::str::contains(
"larger than the manifest's size_bytes",
));
}
#[test]
fn ownership_sync_installs_snapshot_and_preserves_query_behavior() {
let publisher_root = test_env_dir("ownership-sync-publisher");