Skip to content

Commit d25e60c

Browse files
Wenxin-Jiangclaude
andauthored
fix: harden core error handling, blob verification, and force-mode reporting (#56)
- Propagate real I/O and parse errors from read_manifest instead of silently returning Ok(None), so callers see the actual failure reason - Verify downloaded blob content hash before writing to disk, rejecting corrupted or mismatched data at fetch time - Distinguish all-AlreadyPatched from all-NotFound in --force mode, surfacing an informational message when patch files are skipped Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 5960fb5 commit d25e60c

File tree

3 files changed

+49
-7
lines changed

3 files changed

+49
-7
lines changed

crates/socket-patch-core/src/api/blob_fetcher.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,21 @@ async fn download_hashes(
253253

254254
match client.fetch_blob(hash).await {
255255
Ok(Some(data)) => {
256+
// Verify content hash matches expected hash before writing
257+
let actual_hash = crate::hash::git_sha256::compute_git_sha256_from_bytes(&data);
258+
if actual_hash != *hash {
259+
results.push(BlobFetchResult {
260+
hash: hash.clone(),
261+
success: false,
262+
error: Some(format!(
263+
"Content hash mismatch: expected {}, got {}",
264+
hash, actual_hash
265+
)),
266+
});
267+
failed += 1;
268+
continue;
269+
}
270+
256271
let blob_path: PathBuf = blobs_path.join(hash);
257272
match tokio::fs::write(&blob_path, &data).await {
258273
Ok(()) => {

crates/socket-patch-core/src/manifest/operations.rs

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,24 +104,31 @@ pub fn validate_manifest(value: &serde_json::Value) -> Result<PatchManifest, Str
104104
}
105105

106106
/// Read and parse a manifest from the filesystem.
107-
/// Returns Ok(None) if the file does not exist or cannot be parsed.
107+
/// Returns Ok(None) if the file does not exist.
108+
/// Returns Err for I/O errors, JSON parse errors, or validation errors.
108109
pub async fn read_manifest(path: impl AsRef<Path>) -> Result<Option<PatchManifest>, std::io::Error> {
109110
let path = path.as_ref();
110111

111112
let content = match tokio::fs::read_to_string(path).await {
112113
Ok(c) => c,
113114
Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(None),
114-
Err(_) => return Ok(None),
115+
Err(e) => return Err(e), // FIX: propagate actual I/O error
115116
};
116117

117118
let parsed: serde_json::Value = match serde_json::from_str(&content) {
118119
Ok(v) => v,
119-
Err(_) => return Ok(None),
120+
Err(e) => return Err(std::io::Error::new(
121+
std::io::ErrorKind::InvalidData,
122+
format!("Failed to parse manifest JSON: {}", e),
123+
)),
120124
};
121125

122126
match validate_manifest(&parsed) {
123127
Ok(manifest) => Ok(Some(manifest)),
124-
Err(_) => Ok(None),
128+
Err(e) => Err(std::io::Error::new(
129+
std::io::ErrorKind::InvalidData,
130+
e,
131+
)),
125132
}
126133
}
127134

crates/socket-patch-core/src/patch/apply.rs

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -254,13 +254,33 @@ pub async fn apply_package_patch(
254254
result.files_verified.push(verify_result);
255255
}
256256

257-
// Check if all files are already patched (or skipped due to NotFound with force)
258-
let all_patched = result
257+
// Check if all files are already patched
258+
let all_already_patched = result
259+
.files_verified
260+
.iter()
261+
.all(|v| v.status == VerifyStatus::AlreadyPatched);
262+
263+
if all_already_patched {
264+
result.success = true;
265+
return result;
266+
}
267+
268+
// Check if all files are either already patched or not found (force mode skip)
269+
let all_done_or_skipped = result
259270
.files_verified
260271
.iter()
261272
.all(|v| v.status == VerifyStatus::AlreadyPatched || v.status == VerifyStatus::NotFound);
262-
if all_patched {
273+
274+
if all_done_or_skipped {
275+
// Some or all files were not found but skipped via --force
276+
let not_found_count = result.files_verified.iter()
277+
.filter(|v| v.status == VerifyStatus::NotFound)
278+
.count();
263279
result.success = true;
280+
result.error = Some(format!(
281+
"All patch files were skipped: {} not found on disk (--force)",
282+
not_found_count
283+
));
264284
return result;
265285
}
266286

0 commit comments

Comments
 (0)