a73x

10ac0c7f

Make the maximum release size configurable

a73x   2026-08-08 14:43

Commit message
Make the maximum release size configurable
docs/superpowers/plans/2026-08-08-release-packages.md
Old New
@@ -121,6 +121,17 @@ git commit -m "Add shared release name validation"
121 121
122 ### Task 2: Server release store 122 ### Task 2: Server release store
123 123
124 **Execution deviations:**
125 - `list_releases`: fixed a type-inference conflict — the `NotFound` early return forced `versions: Vec<ReleaseVersion>` while the loop later pushed `(SystemTime, ReleaseVersion)` tuples; gave `versions` an explicit tuple type and returned `Vec::new()` directly from the early-return branch.
126 - `finish()`: dropped the no-op `self.temp.flush()` and call `self.temp.as_file().sync_all()?` before persisting, for durability across crashes.
127 - `finish()`: in the force branch only, delete the existing `.sha256` companion before persisting (ignoring `NotFound`), then persist, then write the new companion, so a crash window degrades to "companion missing" rather than "companion wrong"; the no-force branch does not pre-delete, so a rejected duplicate upload leaves the existing artifact's valid companion untouched.
128 - `finish()`: map `persist_noclobber`'s `AlreadyExists` io error to `ReleaseError::AlreadyExists` (was surfacing a raw "os error 17"); `ReleaseUpload` now stores `version` to build the message.
129 - `finish()`: chmod the persisted artifact to `0o644` before persisting (unix-only), so it doesn't keep `NamedTempFile`'s `0600`.
130 - `list_releases`: skip versions whose `files` list is empty, since `begin()` creates the version dir before any bytes arrive and an aborted upload can otherwise phantom-list a zero-file version.
131 - `list_releases`: descending-mtime sort now breaks ties by version name descending, so equal-mtime versions still order lexically-newest-first.
132 - Added `pub fn artifact_path(releases_dir, version, filename) -> Result<PathBuf, ReleaseError>` (validates both segments, strips a single trailing `.sha256` before validating, requires `is_file()`) — **Task 7's HTTP download route must use this instead of hand-joining paths.**
133 - Added a concurrency note to the module doc comment: concurrent force-uploads to the same version/filename aren't serialized; single-writer deployments (current design) are unaffected.
134
124 Filesystem store: atomic uploads with SHA-256, listing, deletion. Lives in the server bin crate. 135 Filesystem store: atomic uploads with SHA-256, listing, deletion. Lives in the server bin crate.
125 136
126 **Files:** 137 **Files:**
src/server/config.rs
Old New
@@ -13,6 +13,8 @@ pub struct ServerConfig {
13 pub authorized_keys: PathBuf, 13 pub authorized_keys: PathBuf,
14 #[serde(default = "default_site_title")] 14 #[serde(default = "default_site_title")]
15 pub site_title: String, 15 pub site_title: String,
16 #[serde(default = "default_max_release_size")]
17 pub max_release_size: u64,
16 } 18 }
17 19
18 fn default_http_bind() -> SocketAddr { 20 fn default_http_bind() -> SocketAddr {
@@ -27,6 +29,10 @@ fn default_site_title() -> String {
27 "git-collab".to_string() 29 "git-collab".to_string()
28 } 30 }
29 31
32 fn default_max_release_size() -> u64 {
33 crate::releases::DEFAULT_MAX_RELEASE_SIZE
34 }
35
30 impl ServerConfig { 36 impl ServerConfig {
31 pub fn from_toml(content: &str) -> Result<Self, toml_edit::de::Error> { 37 pub fn from_toml(content: &str) -> Result<Self, toml_edit::de::Error> {
32 toml_edit::de::from_str(content) 38 toml_edit::de::from_str(content)
@@ -85,6 +91,7 @@ authorized_keys = "/keys"
85 "0.0.0.0:2222".parse::<SocketAddr>().unwrap() 91 "0.0.0.0:2222".parse::<SocketAddr>().unwrap()
86 ); 92 );
87 assert_eq!(config.site_title, "git-collab"); 93 assert_eq!(config.site_title, "git-collab");
94 assert_eq!(config.max_release_size, 1024 * 1024 * 1024);
88 } 95 }
89 96
90 #[test] 97 #[test]
@@ -94,4 +101,15 @@ authorized_keys = "/keys"
94 "#; 101 "#;
95 assert!(ServerConfig::from_toml(toml).is_err()); 102 assert!(ServerConfig::from_toml(toml).is_err());
96 } 103 }
104
105 #[test]
106 fn parse_max_release_size() {
107 let toml = r#"
108 repos_dir = "/srv/git"
109 authorized_keys = "/keys"
110 max_release_size = 1024
111 "#;
112 let config = ServerConfig::from_toml(toml).unwrap();
113 assert_eq!(config.max_release_size, 1024);
114 }
97 } 115 }
src/server/main.rs
Old New
@@ -67,6 +67,7 @@ async fn main() {
67 let ssh_config = ssh::session::SshServerConfig { 67 let ssh_config = ssh::session::SshServerConfig {
68 repos_dir: config.repos_dir.clone(), 68 repos_dir: config.repos_dir.clone(),
69 authorized_keys_path: config.authorized_keys.clone(), 69 authorized_keys_path: config.authorized_keys.clone(),
70 max_release_size: config.max_release_size,
70 }; 71 };
71 72
72 let http_bind = config.http_bind; 73 let http_bind = config.http_bind;
src/server/releases.rs
Old New
@@ -3,6 +3,13 @@
3 //! Layout: `<releases_dir>/<version>/<filename>` plus `<filename>.sha256` 3 //! Layout: `<releases_dir>/<version>/<filename>` plus `<filename>.sha256`
4 //! (sha256sum-compatible: "<hex> <filename>\n"). No manifest — the 4 //! (sha256sum-compatible: "<hex> <filename>\n"). No manifest — the
5 //! filesystem is the index. 5 //! filesystem is the index.
6 //!
7 //! Concurrency note: concurrent SSH sessions are not serialized against
8 //! each other. A single session handles its uploads one at a time, but two
9 //! separate connections can force-upload the same version/filename at the
10 //! same time; the last `finish()` to persist wins and the `.sha256`
11 //! companion may transiently mismatch between the two racing writers. This
12 //! case is accepted and unguarded — there is no locking across sessions.
6 13
7 use std::io::Write; 14 use std::io::Write;
8 use std::path::{Path, PathBuf}; 15 use std::path::{Path, PathBuf};
@@ -48,6 +55,7 @@ pub struct ReleaseUpload {
48 max_size: u64, 55 max_size: u64,
49 dest: PathBuf, 56 dest: PathBuf,
50 sha_dest: PathBuf, 57 sha_dest: PathBuf,
58 version: String,
51 filename: String, 59 filename: String,
52 force: bool, 60 force: bool,
53 } 61 }
@@ -81,6 +89,7 @@ impl ReleaseUpload {
81 max_size, 89 max_size,
82 dest, 90 dest,
83 sha_dest, 91 sha_dest,
92 version: version.to_string(),
84 filename: filename.to_string(), 93 filename: filename.to_string(),
85 force, 94 force,
86 }) 95 })
@@ -97,20 +106,62 @@ impl ReleaseUpload {
97 } 106 }
98 107
99 /// Finalize: atomic rename + write `.sha256`. Returns the hex digest. 108 /// Finalize: atomic rename + write `.sha256`. Returns the hex digest.
100 pub fn finish(mut self) -> Result<String, ReleaseError> { 109 pub fn finish(self) -> Result<String, ReleaseError> {
101 self.temp.flush()?; 110 // Durability: File::flush() is a no-op, so force the bytes to disk
111 // before we rename into place.
112 self.temp.as_file().sync_all()?;
113
114 #[cfg(unix)]
115 {
116 use std::os::unix::fs::PermissionsExt;
117 self.temp
118 .as_file()
119 .set_permissions(std::fs::Permissions::from_mode(0o644))?;
120 }
121
102 let digest = self.hasher.finalize(); 122 let digest = self.hasher.finalize();
103 let hex: String = digest.iter().map(|b| format!("{:02x}", b)).collect(); 123 let hex = format!("{:x}", digest);
124
104 if self.force { 125 if self.force {
126 // The force path is the only one that can replace an existing
127 // artifact, so it's the only one where a stale companion could
128 // end up lying about new bytes. Delete it before persisting: a
129 // crash (or error) between persist and the companion write then
130 // degrades to "companion missing", never "companion wrong". In
131 // the no-force path below, persist_noclobber either fails (dest
132 // untouched, so the existing companion must survive) or
133 // succeeds (dest didn't exist, so the post-persist companion
134 // write below overwrites any orphaned stale companion anyway) —
135 // no pre-delete needed, and pre-deleting there would destroy a
136 // valid companion on a merely-rejected duplicate upload.
137 if let Err(e) = std::fs::remove_file(&self.sha_dest) {
138 if e.kind() != std::io::ErrorKind::NotFound {
139 return Err(e.into());
140 }
141 }
105 self.temp 142 self.temp
106 .persist(&self.dest) 143 .persist(&self.dest)
107 .map_err(|e| ReleaseError::Io(e.error))?; 144 .map_err(|e| ReleaseError::Io(e.error))?;
108 } else { 145 } else {
109 self.temp 146 self.temp.persist_noclobber(&self.dest).map_err(|e| {
110 .persist_noclobber(&self.dest) 147 if e.error.kind() == std::io::ErrorKind::AlreadyExists {
111 .map_err(|e| ReleaseError::Io(e.error))?; 148 ReleaseError::AlreadyExists(format!("{}/{}", self.version, self.filename))
149 } else {
150 ReleaseError::Io(e.error)
151 }
152 })?;
112 } 153 }
113 std::fs::write(&self.sha_dest, format!("{} {}\n", hex, self.filename))?; 154 std::fs::write(&self.sha_dest, format!("{} {}\n", hex, self.filename))?;
155 // std::fs::write() creates the file honoring the process umask, so
156 // under a restrictive umask (e.g. 077) the companion could end up
157 // 0600 while the artifact above is explicitly forced to 0644.
158 // Force an explicit mode so the two are consistent regardless of
159 // umask.
160 #[cfg(unix)]
161 {
162 use std::os::unix::fs::PermissionsExt;
163 std::fs::set_permissions(&self.sha_dest, std::fs::Permissions::from_mode(0o644))?;
164 }
114 Ok(hex) 165 Ok(hex)
115 } 166 }
116 } 167 }
@@ -180,10 +231,16 @@ pub fn list_releases(releases_dir: &Path) -> Result<ReleaseIndex, ReleaseError>
180 files.push(ReleaseFile { name, size, sha256 }); 231 files.push(ReleaseFile { name, size, sha256 });
181 } 232 }
182 files.sort_by(|a, b| a.name.cmp(&b.name)); 233 files.sort_by(|a, b| a.name.cmp(&b.name));
234 // begin() creates the version directory before any bytes have
235 // arrived, so an aborted upload can leave an empty version dir.
236 // Don't list phantom versions with zero files.
237 if files.is_empty() {
238 continue;
239 }
183 versions.push((mtime, ReleaseVersion { version, published, files })); 240 versions.push((mtime, ReleaseVersion { version, published, files }));
184 } 241 }
185 242
186 versions.sort_by(|a, b| b.0.cmp(&a.0).then_with(|| a.1.version.cmp(&b.1.version))); 243 versions.sort_by(|a, b| b.0.cmp(&a.0).then_with(|| b.1.version.cmp(&a.1.version)));
187 Ok(ReleaseIndex { 244 Ok(ReleaseIndex {
188 versions: versions.into_iter().map(|(_, v)| v).collect(), 245 versions: versions.into_iter().map(|(_, v)| v).collect(),
189 }) 246 })
@@ -226,6 +283,28 @@ pub fn delete_release(
226 Ok(()) 283 Ok(())
227 } 284 }
228 285
286 /// Resolve an artifact (or `.sha256` companion) path safely. Validates both
287 /// segments; the filename may carry a single ".sha256" suffix. Returns
288 /// `NotFound` if the file does not exist.
289 pub fn artifact_path(
290 releases_dir: &Path,
291 version: &str,
292 filename: &str,
293 ) -> Result<PathBuf, ReleaseError> {
294 if !validate_name(version) {
295 return Err(ReleaseError::InvalidName(version.to_string()));
296 }
297 let base_name = filename.strip_suffix(".sha256").unwrap_or(filename);
298 if !validate_name(base_name) {
299 return Err(ReleaseError::InvalidName(filename.to_string()));
300 }
301 let path = releases_dir.join(version).join(filename);
302 if !path.is_file() {
303 return Err(ReleaseError::NotFound(format!("{}/{}", version, filename)));
304 }
305 Ok(path)
306 }
307
229 #[cfg(test)] 308 #[cfg(test)]
230 mod tests { 309 mod tests {
231 use super::*; 310 use super::*;
@@ -255,6 +334,22 @@ mod tests {
255 ); 334 );
256 } 335 }
257 336
337 #[cfg(unix)]
338 #[test]
339 fn companion_mode_is_explicit_0644() {
340 use std::os::unix::fs::PermissionsExt;
341
342 let tmp = TempDir::new().unwrap();
343 upload(tmp.path(), "v1", "a.tar.gz", b"x", false).unwrap();
344
345 // finish() sets an explicit mode on the companion (rather than
346 // relying on std::fs::write()'s umask-dependent default), so this
347 // holds regardless of the process umask.
348 let sha_file = tmp.path().join("v1").join("a.tar.gz.sha256");
349 let mode = std::fs::metadata(&sha_file).unwrap().permissions().mode();
350 assert_eq!(mode & 0o777, 0o644);
351 }
352
258 #[test] 353 #[test]
259 fn duplicate_upload_rejected_without_force() { 354 fn duplicate_upload_rejected_without_force() {
260 let tmp = TempDir::new().unwrap(); 355 let tmp = TempDir::new().unwrap();
@@ -269,17 +364,29 @@ mod tests {
269 } 364 }
270 365
271 #[test] 366 #[test]
367 fn rejected_duplicate_keeps_existing_companion() {
368 let tmp = TempDir::new().unwrap();
369 let original_sha = upload(tmp.path(), "v1", "a.tar.gz", b"one", false).unwrap();
370 let err = upload(tmp.path(), "v1", "a.tar.gz", b"two", false).unwrap_err();
371 assert!(matches!(err, ReleaseError::AlreadyExists(_)));
372
373 let companion = std::fs::read_to_string(tmp.path().join("v1").join("a.tar.gz.sha256"))
374 .expect("companion must still exist after a rejected duplicate upload");
375 assert_eq!(companion, format!("{} a.tar.gz\n", original_sha));
376 }
377
378 #[test]
272 fn force_replaces_file_and_checksum() { 379 fn force_replaces_file_and_checksum() {
273 let tmp = TempDir::new().unwrap(); 380 let tmp = TempDir::new().unwrap();
274 upload(tmp.path(), "v1", "a.tar.gz", b"one", false).unwrap(); 381 upload(tmp.path(), "v1", "a.tar.gz", b"one", false).unwrap();
275 let sha2 = upload(tmp.path(), "v1", "a.tar.gz", b"two", true).unwrap(); 382 let sha_after = upload(tmp.path(), "v1", "a.tar.gz", b"two", true).unwrap();
276 assert_eq!( 383 assert_eq!(
277 std::fs::read(tmp.path().join("v1").join("a.tar.gz")).unwrap(), 384 std::fs::read(tmp.path().join("v1").join("a.tar.gz")).unwrap(),
278 b"two" 385 b"two"
279 ); 386 );
280 assert!(std::fs::read_to_string(tmp.path().join("v1").join("a.tar.gz.sha256")) 387 assert!(std::fs::read_to_string(tmp.path().join("v1").join("a.tar.gz.sha256"))
281 .unwrap() 388 .unwrap()
282 .starts_with(&sha2)); 389 .starts_with(&sha_after));
283 } 390 }
284 391
285 #[test] 392 #[test]
@@ -380,4 +487,116 @@ mod tests {
380 Err(ReleaseError::NotFound(_)) 487 Err(ReleaseError::NotFound(_))
381 )); 488 ));
382 } 489 }
490
491 #[test]
492 fn multi_chunk_upload_streams_correctly() {
493 let tmp = TempDir::new().unwrap();
494 let mut up = ReleaseUpload::begin(
495 tmp.path(),
496 "v1",
497 "app.tar.gz",
498 false,
499 DEFAULT_MAX_RELEASE_SIZE,
500 )
501 .unwrap();
502 up.write(b"hel").unwrap();
503 up.write(b"lo, ").unwrap();
504 up.write(b"world").unwrap();
505 let sha = up.finish().unwrap();
506
507 let expected = format!("{:x}", Sha256::digest(b"hello, world"));
508 assert_eq!(sha, expected);
509 assert_eq!(
510 std::fs::read(tmp.path().join("v1").join("app.tar.gz")).unwrap(),
511 b"hello, world"
512 );
513 }
514
515 #[test]
516 fn exact_size_boundary_accepted() {
517 let tmp = TempDir::new().unwrap();
518 let mut up = ReleaseUpload::begin(tmp.path(), "v1", "a.tar.gz", false, 5).unwrap();
519 assert!(up.write(b"12345").is_ok());
520 let sha = up.finish().unwrap();
521 assert_eq!(
522 std::fs::read(tmp.path().join("v1").join("a.tar.gz")).unwrap(),
523 b"12345"
524 );
525 assert_eq!(sha.len(), 64);
526 }
527
528 #[test]
529 fn orphaned_companion_yields_empty_sha256() {
530 let tmp = TempDir::new().unwrap();
531 upload(tmp.path(), "v1", "a.tar.gz", b"x", false).unwrap();
532 std::fs::remove_file(tmp.path().join("v1").join("a.tar.gz.sha256")).unwrap();
533
534 let index = list_releases(tmp.path()).unwrap();
535 assert_eq!(index.versions[0].files[0].sha256, "");
536 }
537
538 #[test]
539 fn empty_version_dir_not_listed() {
540 let tmp = TempDir::new().unwrap();
541 std::fs::create_dir_all(tmp.path().join("v1-empty")).unwrap();
542 upload(tmp.path(), "v2", "a.tar.gz", b"x", false).unwrap();
543
544 let index = list_releases(tmp.path()).unwrap();
545 assert_eq!(index.versions.len(), 1);
546 assert_eq!(index.versions[0].version, "v2");
547 }
548
549 #[test]
550 fn concurrent_dest_creation_maps_to_already_exists() {
551 let tmp = TempDir::new().unwrap();
552 let mut up = ReleaseUpload::begin(
553 tmp.path(),
554 "v1",
555 "a.tar.gz",
556 false,
557 DEFAULT_MAX_RELEASE_SIZE,
558 )
559 .unwrap();
560 up.write(b"race").unwrap();
561 // Simulate another writer creating the dest file after begin()
562 // checked for its absence but before this upload persists.
563 std::fs::write(tmp.path().join("v1").join("a.tar.gz"), b"other").unwrap();
564
565 let err = up.finish().unwrap_err();
566 assert!(matches!(err, ReleaseError::AlreadyExists(_)));
567 }
568
569 #[test]
570 fn artifact_path_resolves_existing_and_companion() {
571 let tmp = TempDir::new().unwrap();
572 upload(tmp.path(), "v1", "a.tar.gz", b"x", false).unwrap();
573
574 let file = artifact_path(tmp.path(), "v1", "a.tar.gz").unwrap();
575 assert_eq!(file, tmp.path().join("v1").join("a.tar.gz"));
576
577 let companion = artifact_path(tmp.path(), "v1", "a.tar.gz.sha256").unwrap();
578 assert_eq!(companion, tmp.path().join("v1").join("a.tar.gz.sha256"));
579 }
580
581 #[test]
582 fn artifact_path_rejects_traversal() {
583 let tmp = TempDir::new().unwrap();
584 assert!(matches!(
585 artifact_path(tmp.path(), "../v1", "a.tar.gz"),
586 Err(ReleaseError::InvalidName(_))
587 ));
588 assert!(matches!(
589 artifact_path(tmp.path(), "v1", "../../a.tar.gz"),
590 Err(ReleaseError::InvalidName(_))
591 ));
592 }
593
594 #[test]
595 fn artifact_path_missing_is_not_found() {
596 let tmp = TempDir::new().unwrap();
597 assert!(matches!(
598 artifact_path(tmp.path(), "v1", "nope.tar.gz"),
599 Err(ReleaseError::NotFound(_))
600 ));
601 }
383 } 602 }
src/server/ssh/session.rs
Old New
@@ -17,6 +17,7 @@ use super::auth::{is_authorized, load_authorized_keys};
17 pub struct SshServerConfig { 17 pub struct SshServerConfig {
18 pub repos_dir: PathBuf, 18 pub repos_dir: PathBuf,
19 pub authorized_keys_path: PathBuf, 19 pub authorized_keys_path: PathBuf,
20 pub max_release_size: u64,
20 } 21 }
21 22
22 /// Per-connection SSH session handler. 23 /// Per-connection SSH session handler.