a73x

1a471c20

Serve release listings and downloads over HTTP

a73x   2026-08-08 15:30

Commit message
Serve release listings and downloads over HTTP
docs/superpowers/plans/2026-08-08-release-packages.md
Old New
@@ -1597,12 +1597,24 @@ git commit -m "Handle collab-release upload/list/delete over SSH"
1597 - Amendment B applied: `ExecCommand::repo(&self) -> &str` is used for the `resolve_repo_path` call instead of re-matching. 1597 - Amendment B applied: `ExecCommand::repo(&self) -> &str` is used for the `resolve_repo_path` call instead of re-matching.
1598 - Release dispatch is an early return (`let git_cmd = match exec_cmd { Git { cmd, .. } => cmd, Release(rel) => { self.handle_release_command(..); return Ok(()); } };`) rather than wrapping the whole git flow in a `match` arm — behaviorally identical, avoids re-indenting the untouched git flow. 1598 - Release dispatch is an early return (`let git_cmd = match exec_cmd { Git { cmd, .. } => cmd, Release(rel) => { self.handle_release_command(..); return Ok(()); } };`) rather than wrapping the whole git flow in a `match` arm — behaviorally identical, avoids re-indenting the untouched git flow.
1599 - `UploadState::Active` holds a `Box<ReleaseUpload>` to satisfy clippy's `large_enum_variant`. 1599 - `UploadState::Active` holds a `Box<ReleaseUpload>` to satisfy clippy's `large_enum_variant`.
1600 - `src/server/ssh/session.rs` and `tests/release_server_test.rs` were run through `rustfmt` (both had pre-existing drift; `src/server/releases.rs` and `tests/common/mod.rs` still do and were left alone). 1600 - `src/server/ssh/session.rs` and `tests/release_server_test.rs` were run through `rustfmt` (both had pre-existing drift; `src/server/releases.rs` still does and was left alone).
1601
1602 **Review follow-ups (applied after Task 6's first commit):**
1603
1604 - `stdin_tx` is now `Option<(ChannelId, mpsc::Sender<Vec<u8>>)>`: `data()` forwards and `channel_eof` tears down only on a channel match, so an EOF on a release channel can't close a concurrent git push's stdin under SSH connection multiplexing.
1605 - Unauthorized release commands reply with the same `error: repository not found` as unknown repos (distinct `warn!` logs retained), matching the HTTP layer's deliberate 404-collapsing so errors can't probe which private repos exist.
1606 - `write_policy_gates_upload_and_delete_but_not_list` now seeds a release before applying the restrictive policy, and asserts the seed survives (list + on-disk) and the denied upload left nothing — the read-only half was previously vacuous.
1607 - Added `git_push_and_clone_over_ssh` (real `git push`/`git clone` through the harness key), covering `exec_request`'s git branch, stdin forwarding and `channel_eof` end to end. Release e2e file is 9 tests total.
1608 - Added `Handler::channel_close` to drop an unfinished upload (temp cleanup for clients that close without EOF); list serialization failure now replies with an error + exit 1 instead of a silent empty index; the four inline git-branch rejection sequences fold into `reply_and_close`; `GitCmd::as_str(self)` takes self by value; `handle_release_command` documents the accepted synchronous-I/O-on-runtime tradeoff.
1609 - Separate commit: `src/server/main.rs` now `std::process::exit(1)`s when either listener future finishes (a bind failure previously fell out of `main` and exited **0**, which is the flake seen during Task 6). Harness `new_with_extra_config` retries startup once with fresh ports, and readiness failures now include the server's captured stderr. Verified by hand: a busy SSH port yields `exit code 1` + `SSH server error: Address already in use`, and a forced first-attempt port collision is transparently retried.
1601 1610
1602 --- 1611 ---
1603 1612
1604 ### Task 7: HTTP releases page and downloads 1613 ### Task 7: HTTP releases page and downloads
1605 1614
1615 **Execution deviations:**
1616 - `release_download`: per Task 2's note, used `crate::releases::artifact_path(&releases_dir(&entry), &version, &filename)` instead of the plan's manual `validate_name`/`strip_suffix`/`join` block — `Err` maps to the plain 404, `Ok(path)` is opened directly; the `git_collab::release::validate_name` import was dropped from `releases.rs` since it's no longer called there.
1617
1606 **Files:** 1618 **Files:**
1607 - Modify: `tests/release_server_test.rs` (tests first) 1619 - Modify: `tests/release_server_test.rs` (tests first)
1608 - Create: `src/server/http/repo/releases.rs` 1620 - Create: `src/server/http/repo/releases.rs`
src/server/http/mod.rs
Old New
@@ -37,6 +37,11 @@ pub fn router(state: AppState) -> Router {
37 "/{repo_name}/issues/{id}", 37 "/{repo_name}/issues/{id}",
38 axum::routing::get(repo::issue_detail), 38 axum::routing::get(repo::issue_detail),
39 ) 39 )
40 .route("/{repo_name}/releases", axum::routing::get(repo::releases))
41 .route(
42 "/{repo_name}/releases/{version}/{filename}",
43 axum::routing::get(repo::release_download),
44 )
40 .route( 45 .route(
41 "/{repo_dot_git}/info/refs", 46 "/{repo_dot_git}/info/refs",
42 axum::routing::get(git_http::info_refs), 47 axum::routing::get(git_http::info_refs),
src/server/http/repo/mod.rs
Old New
@@ -3,6 +3,7 @@ mod diff;
3 mod issues; 3 mod issues;
4 mod overview; 4 mod overview;
5 mod patches; 5 mod patches;
6 mod releases;
6 mod tree; 7 mod tree;
7 8
8 pub use commits::{commits, commits_ref}; 9 pub use commits::{commits, commits_ref};
@@ -10,6 +11,7 @@ pub use diff::diff;
10 pub use issues::{issue_detail, issues}; 11 pub use issues::{issue_detail, issues};
11 pub use overview::overview; 12 pub use overview::overview;
12 pub use patches::{patch_detail, patches}; 13 pub use patches::{patch_detail, patches};
14 pub use releases::{release_download, releases};
13 pub use tree::{blob, tree, tree_root}; 15 pub use tree::{blob, tree, tree_root};
14 16
15 use axum::http::StatusCode; 17 use axum::http::StatusCode;
src/server/http/repo/releases.rs
Old New
@@ -0,0 +1,90 @@
1 use std::sync::Arc;
2
3 use axum::extract::{Path, State};
4 use axum::http::{header, HeaderValue, StatusCode};
5 use axum::response::{IntoResponse, Response};
6 use tokio_util::io::ReaderStream;
7
8 use super::{collab_counts, open_repo, AppState};
9 use crate::releases::{list_releases, releases_dir, ReleaseVersion};
10
11 #[derive(askama::Template, askama_web::WebTemplate)]
12 #[template(path = "releases.html")]
13 pub struct ReleasesTemplate {
14 pub site_title: String,
15 pub repo_name: String,
16 pub active_section: String,
17 pub open_patches: usize,
18 pub open_issues: usize,
19 pub versions: Vec<ReleaseVersion>,
20 }
21
22 pub async fn releases(
23 Path(repo_name): Path<String>,
24 State(state): State<Arc<AppState>>,
25 ) -> Response {
26 let (entry, repo) = match open_repo(&state, &repo_name) {
27 Ok(pair) => pair,
28 Err(resp) => return resp,
29 };
30 let (open_patches, open_issues) = collab_counts(&repo);
31 let versions = list_releases(&releases_dir(&entry))
32 .map(|index| index.versions)
33 .unwrap_or_default();
34
35 ReleasesTemplate {
36 site_title: state.site_title.clone(),
37 repo_name,
38 active_section: "releases".to_string(),
39 open_patches,
40 open_issues,
41 versions,
42 }
43 .into_response()
44 }
45
46 pub async fn release_download(
47 Path((repo_name, version, filename)): Path<(String, String, String)>,
48 State(state): State<Arc<AppState>>,
49 ) -> Response {
50 let entry = match crate::repos::resolve(&state.repos_dir, &repo_name) {
51 Some(e) => e,
52 None => return plain_404(),
53 };
54 // Downloads are data distribution, like clone.
55 if !entry.policy.allows_anonymous_http() {
56 return plain_404();
57 }
58
59 let path = match crate::releases::artifact_path(&releases_dir(&entry), &version, &filename) {
60 Ok(p) => p,
61 Err(_) => return plain_404(),
62 };
63
64 let file = match tokio::fs::File::open(&path).await {
65 Ok(f) => f,
66 Err(_) => return plain_404(),
67 };
68 let len = match file.metadata().await {
69 Ok(m) if m.is_file() => m.len(),
70 _ => return plain_404(),
71 };
72
73 let content_type = if filename.ends_with(".sha256") {
74 "text/plain; charset=utf-8"
75 } else {
76 "application/octet-stream"
77 };
78
79 let mut response = Response::new(axum::body::Body::from_stream(ReaderStream::new(file)));
80 let headers = response.headers_mut();
81 headers.insert(header::CONTENT_TYPE, HeaderValue::from_static(content_type));
82 if let Ok(value) = HeaderValue::from_str(&len.to_string()) {
83 headers.insert(header::CONTENT_LENGTH, value);
84 }
85 response
86 }
87
88 fn plain_404() -> Response {
89 (StatusCode::NOT_FOUND, "Not found").into_response()
90 }
src/server/http/templates/releases.html
Old New
@@ -0,0 +1,26 @@
1 {% extends "repo_base.html" %}
2
3 {% block title %}Releases — {{ repo_name }} — {{ site_title }}{% endblock %}
4
5 {% block content %}
6 <h2>Releases</h2>
7 {% if versions.is_empty() %}
8 <p style="color: #666;">No releases.</p>
9 {% else %}
10 {% for v in versions %}
11 <section>
12 <h3>{{ v.version }}</h3>
13 <p style="color: #666;">{{ v.published }}</p>
14 <ul>
15 {% for f in v.files %}
16 <li>
17 <a href="/{{ repo_name }}/releases/{{ v.version }}/{{ f.name }}">{{ f.name }}</a>
18 ({{ f.size }} bytes)
19 <code>{{ f.sha256 }}</code>
20 </li>
21 {% endfor %}
22 </ul>
23 </section>
24 {% endfor %}
25 {% endif %}
26 {% endblock %}
src/server/http/templates/repo_base.html
Old New
@@ -7,6 +7,7 @@
7 <a href="/{{ repo_name }}/tree"{% if active_section == "tree" %} class="active"{% endif %}>tree</a> 7 <a href="/{{ repo_name }}/tree"{% if active_section == "tree" %} class="active"{% endif %}>tree</a>
8 <a href="/{{ repo_name }}/patches"{% if active_section == "patches" %} class="active"{% endif %}>patches ({{ open_patches }})</a> 8 <a href="/{{ repo_name }}/patches"{% if active_section == "patches" %} class="active"{% endif %}>patches ({{ open_patches }})</a>
9 <a href="/{{ repo_name }}/issues"{% if active_section == "issues" %} class="active"{% endif %}>issues ({{ open_issues }})</a> 9 <a href="/{{ repo_name }}/issues"{% if active_section == "issues" %} class="active"{% endif %}>issues ({{ open_issues }})</a>
10 <a href="/{{ repo_name }}/releases"{% if active_section == "releases" %} class="active"{% endif %}>releases</a>
10 </nav> 11 </nav>
11 {% endblock %} 12 {% endblock %}
12 13
src/server/main.rs
Old New
@@ -84,16 +84,23 @@ async fn main() {
84 let http_fut = axum::serve(http_listener, router); 84 let http_fut = axum::serve(http_listener, router);
85 let ssh_fut = ssh::serve(ssh_bind, host_key, ssh_config); 85 let ssh_fut = ssh::serve(ssh_bind, host_key, ssh_config);
86 86
87 // Either listener finishing means the server is no longer serving what it
88 // was asked to serve, so exit non-zero. Falling out of main here would
89 // exit 0 and look like a clean shutdown to supervisors and test harnesses.
87 tokio::select! { 90 tokio::select! {
88 result = http_fut => { 91 result = http_fut => {
89 if let Err(e) = result { 92 if let Err(e) = result {
90 eprintln!("HTTP server error: {}", e); 93 eprintln!("HTTP server error: {}", e);
91 } 94 }
95 eprintln!("HTTP listener stopped; shutting down");
96 std::process::exit(1);
92 } 97 }
93 result = ssh_fut => { 98 result = ssh_fut => {
94 if let Err(e) = result { 99 if let Err(e) = result {
95 eprintln!("SSH server error: {}", e); 100 eprintln!("SSH server error: {}", e);
96 } 101 }
102 eprintln!("SSH listener stopped; shutting down");
103 std::process::exit(1);
97 } 104 }
98 } 105 }
99 } 106 }
src/server/ssh/session.rs
Old New
@@ -24,8 +24,12 @@ pub struct SshServerConfig {
24 pub struct SshHandler { 24 pub struct SshHandler {
25 config: Arc<SshServerConfig>, 25 config: Arc<SshServerConfig>,
26 authenticated_principal: Option<String>, 26 authenticated_principal: Option<String>,
27 /// Sender for forwarding client data (stdin) to the spawned git subprocess. 27 /// Sender for forwarding client data (stdin) to the spawned git
28 stdin_tx: Option<mpsc::Sender<Vec<u8>>>, 28 /// subprocess, tagged with the channel that owns it. SSH connections can
29 /// multiplex channels, so both routing and teardown must match on it:
30 /// otherwise an EOF on a release channel would close a concurrent git
31 /// push's stdin.
32 stdin_tx: Option<(ChannelId, mpsc::Sender<Vec<u8>>)>,
29 /// In-progress release upload, fed by data() and finalized on channel EOF. 33 /// In-progress release upload, fed by data() and finalized on channel EOF.
30 active_upload: Option<UploadSession>, 34 active_upload: Option<UploadSession>,
31 } 35 }
@@ -52,6 +56,12 @@ impl SshHandler {
52 } 56 }
53 } 57 }
54 58
59 /// Handle a release verb. The store calls here (`begin`, `list_releases`,
60 /// `delete_release`) are synchronous filesystem I/O on the async runtime.
61 /// That is an accepted tradeoff at this server's scale: the operations are
62 /// metadata-sized directory walks and renames, not the artifact streaming
63 /// itself (upload bytes arrive incrementally via `data()`). If release
64 /// counts or concurrency grow, move these onto `spawn_blocking`.
55 fn handle_release_command( 65 fn handle_release_command(
56 &mut self, 66 &mut self,
57 channel: ChannelId, 67 channel: ChannelId,
@@ -60,12 +70,17 @@ impl SshHandler {
60 resolved_path: &Path, 70 resolved_path: &Path,
61 principal: &str, 71 principal: &str,
62 ) { 72 ) {
73 // Unknown repo and unauthorized repo get the SAME reply, so the error
74 // can't be used to probe which private repos exist. This mirrors the
75 // deliberate 404-collapsing in the HTTP layer (see git_http.rs).
76 const NOT_FOUND: &str = "error: repository not found\n";
77
63 // Releases never auto-create a repo (unlike git-receive-pack). 78 // Releases never auto-create a repo (unlike git-receive-pack).
64 let entry = match crate::repos::entry_for_path(resolved_path) { 79 let entry = match crate::repos::entry_for_path(resolved_path) {
65 Some(entry) => entry, 80 Some(entry) => entry,
66 None => { 81 None => {
67 warn!("Rejected release command: unknown repo {:?}", resolved_path); 82 warn!("Rejected release command: unknown repo {:?}", resolved_path);
68 reply_and_close(session, channel, "error: repository not found\n", 1); 83 reply_and_close(session, channel, NOT_FOUND, 1);
69 return; 84 return;
70 } 85 }
71 }; 86 };
@@ -81,7 +96,7 @@ impl SshHandler {
81 "Rejected release command: principal {} not authorized on {:?}", 96 "Rejected release command: principal {} not authorized on {:?}",
82 principal, resolved_path 97 principal, resolved_path
83 ); 98 );
84 reply_and_close(session, channel, "error: not authorized\n", 1); 99 reply_and_close(session, channel, NOT_FOUND, 1);
85 return; 100 return;
86 } 101 }
87 102
@@ -111,11 +126,15 @@ impl SshHandler {
111 } 126 }
112 }, 127 },
113 ReleaseCmd::List { .. } => match crate::releases::list_releases(&dir) { 128 ReleaseCmd::List { .. } => match crate::releases::list_releases(&dir) {
114 Ok(index) => { 129 Ok(index) => match serde_json::to_string_pretty(&index) {
115 let json = serde_json::to_string_pretty(&index) 130 Ok(json) => reply_and_close(session, channel, &format!("{}\n", json), 0),
116 .unwrap_or_else(|_| "{\"versions\":[]}".to_string()); 131 Err(e) => {
117 reply_and_close(session, channel, &format!("{}\n", json), 0); 132 // Never answer a list with a silent empty index: that
118 } 133 // would look like "no releases" to the client.
134 error!("Failed to serialize release index for {:?}: {}", dir, e);
135 reply_and_close(session, channel, "error: failed to encode index\n", 1);
136 }
137 },
119 Err(e) => { 138 Err(e) => {
120 reply_and_close(session, channel, &format!("error: {}\n", e), 1); 139 reply_and_close(session, channel, &format!("error: {}\n", e), 1);
121 } 140 }
@@ -163,8 +182,10 @@ pub fn parse_git_command(data: &str) -> Option<(&str, &str)> {
163 } 182 }
164 183
165 /// The only two git binaries this server will ever spawn. Keeping this an 184 /// The only two git binaries this server will ever spawn. Keeping this an
166 /// enum (rather than a String lifted from the client's exec request) makes 185 /// enum, rather than a String lifted from the client's exec request, means
167 /// "spawn an attacker-named binary" unrepresentable. 186 /// the *name* we spawn is constrained to these two constants regardless of
187 /// what the client sent. (Which binary those names resolve to is still a
188 /// function of the server process's `PATH`.)
168 #[derive(Debug, Clone, Copy, PartialEq, Eq)] 189 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
169 pub enum GitCmd { 190 pub enum GitCmd {
170 UploadPack, 191 UploadPack,
@@ -172,7 +193,7 @@ pub enum GitCmd {
172 } 193 }
173 194
174 impl GitCmd { 195 impl GitCmd {
175 pub fn as_str(&self) -> &'static str { 196 pub fn as_str(self) -> &'static str {
176 match self { 197 match self {
177 GitCmd::UploadPack => "git-upload-pack", 198 GitCmd::UploadPack => "git-upload-pack",
178 GitCmd::ReceivePack => "git-receive-pack", 199 GitCmd::ReceivePack => "git-receive-pack",
@@ -482,9 +503,7 @@ impl Handler for SshHandler {
482 "Rejected exec request: path is not a git repo: {:?}", 503 "Rejected exec request: path is not a git repo: {:?}",
483 resolved_path 504 resolved_path
484 ); 505 );
485 session.exit_status_request(channel, 1); 506 reply_and_close(session, channel, "", 1);
486 session.eof(channel);
487 session.close(channel);
488 return Ok(()); 507 return Ok(());
489 } 508 }
490 }; 509 };
@@ -501,9 +520,7 @@ impl Handler for SshHandler {
501 git_cmd.as_str(), 520 git_cmd.as_str(),
502 resolved_path 521 resolved_path
503 ); 522 );
504 session.exit_status_request(channel, 1); 523 reply_and_close(session, channel, "", 1);
505 session.eof(channel);
506 session.close(channel);
507 return Ok(()); 524 return Ok(());
508 } 525 }
509 } else { 526 } else {
@@ -516,24 +533,22 @@ impl Handler for SshHandler {
516 "Rejected exec request: repo path does not exist: {:?}", 533 "Rejected exec request: repo path does not exist: {:?}",
517 resolved_path 534 resolved_path
518 ); 535 );
519 session.exit_status_request(channel, 1); 536 reply_and_close(session, channel, "", 1);
520 session.eof(channel);
521 session.close(channel);
522 return Ok(()); 537 return Ok(());
523 } 538 }
524 Err(e) => { 539 Err(e) => {
525 error!("Failed to create repo {:?}: {}", resolved_path, e); 540 error!("Failed to create repo {:?}: {}", resolved_path, e);
526 session.exit_status_request(channel, 1); 541 reply_and_close(session, channel, "", 1);
527 session.eof(channel);
528 session.close(channel);
529 return Ok(()); 542 return Ok(());
530 } 543 }
531 } 544 }
532 } 545 }
533 546
534 // Create a channel for forwarding client stdin data to the git subprocess 547 // Create a channel for forwarding client stdin data to the git
548 // subprocess. Tagged with the owning SSH channel so data routing and
549 // EOF teardown can't cross-talk between multiplexed channels.
535 let (tx, rx) = mpsc::channel::<Vec<u8>>(64); 550 let (tx, rx) = mpsc::channel::<Vec<u8>>(64);
536 self.stdin_tx = Some(tx); 551 self.stdin_tx = Some((channel, tx));
537 552
538 // Spawn the git subprocess 553 // Spawn the git subprocess
539 let handle = session.handle(); 554 let handle = session.handle();
@@ -567,9 +582,10 @@ impl Handler for SshHandler {
567 return Ok(()); 582 return Ok(());
568 } 583 }
569 } 584 }
570 // Forward client data to the git subprocess's stdin 585 // Forward client data to the git subprocess's stdin, but only for the
571 if let Some(ref tx) = self.stdin_tx { 586 // channel that subprocess belongs to.
572 if tx.send(data.to_vec()).await.is_err() { 587 if let Some((git_channel, tx)) = self.stdin_tx.as_ref() {
588 if *git_channel == channel && tx.send(data.to_vec()).await.is_err() {
573 debug!("stdin channel closed, dropping data"); 589 debug!("stdin channel closed, dropping data");
574 } 590 }
575 } 591 }
@@ -581,8 +597,11 @@ impl Handler for SshHandler {
581 channel: ChannelId, 597 channel: ChannelId,
582 session: &mut Session, 598 session: &mut Session,
583 ) -> Result<(), Self::Error> { 599 ) -> Result<(), Self::Error> {
584 // Close the git subprocess's stdin, if any. 600 // Close the git subprocess's stdin, but only if this EOF is for the
585 self.stdin_tx = None; 601 // channel that owns it.
602 if matches!(self.stdin_tx.as_ref(), Some((git_channel, _)) if *git_channel == channel) {
603 self.stdin_tx = None;
604 }
586 605
587 if let Some(upload) = self.active_upload.take() { 606 if let Some(upload) = self.active_upload.take() {
588 if upload.channel != channel { 607 if upload.channel != channel {
@@ -601,6 +620,29 @@ impl Handler for SshHandler {
601 } 620 }
602 Ok(()) 621 Ok(())
603 } 622 }
623
624 async fn channel_close(
625 &mut self,
626 channel: ChannelId,
627 _session: &mut Session,
628 ) -> Result<(), Self::Error> {
629 // A close without a preceding EOF still has to release the git child's
630 // stdin, or it stays open until connection teardown.
631 if matches!(self.stdin_tx.as_ref(), Some((git_channel, _)) if *git_channel == channel) {
632 self.stdin_tx = None;
633 }
634
635 // A client that closes without EOF gets no reply, but the half-written
636 // temp file must still go away. Dropping the ReleaseUpload deletes it.
637 if let Some(upload) = self.active_upload.take() {
638 if upload.channel != channel {
639 self.active_upload = Some(upload);
640 } else {
641 debug!("Channel closed with an unfinished upload; discarding temp file");
642 }
643 }
644 Ok(())
645 }
604 } 646 }
605 647
606 /// Send a final message (if any), an exit status, and close the channel. 648 /// Send a final message (if any), an exit status, and close the channel.
tests/common/mod.rs
Old New
@@ -530,39 +530,55 @@ impl ServerHarness {
530 let authorized_keys = root.path().join("authorized_keys"); 530 let authorized_keys = root.path().join("authorized_keys");
531 std::fs::write(&authorized_keys, "").unwrap(); 531 std::fs::write(&authorized_keys, "").unwrap();
532 532
533 let http_addr = pick_loopback_addr();
534 let ssh_addr = pick_loopback_addr();
535 let config_path = root.path().join("server.toml"); 533 let config_path = root.path().join("server.toml");
536 std::fs::write(
537 &config_path,
538 format!(
539 "repos_dir = {:?}\nhttp_bind = \"{}\"\nssh_bind = \"{}\"\nauthorized_keys = {:?}\nsite_title = \"git-collab test\"\n{}",
540 repos_dir,
541 http_addr,
542 ssh_addr,
543 authorized_keys,
544 extra_config,
545 ),
546 )
547 .unwrap();
548 534
549 let server = Command::new(env!("CARGO_BIN_EXE_git-collab-server")) 535 // Ports come from pick_loopback_addr's bind-and-drop, so another
550 .args(["--config", config_path.to_str().unwrap()]) 536 // process can steal one between the probe and the server's bind.
551 .stdout(Stdio::piped()) 537 // Retry once with fresh ports before failing the test.
552 .stderr(Stdio::piped()) 538 let mut last_error = String::new();
553 .spawn() 539 for attempt in 1..=2 {
554 .expect("failed to start git-collab-server"); 540 let http_addr = pick_loopback_addr();
541 let ssh_addr = pick_loopback_addr();
542 std::fs::write(
543 &config_path,
544 format!(
545 "repos_dir = {:?}\nhttp_bind = \"{}\"\nssh_bind = \"{}\"\nauthorized_keys = {:?}\nsite_title = \"git-collab test\"\n{}",
546 repos_dir,
547 http_addr,
548 ssh_addr,
549 authorized_keys,
550 extra_config,
551 ),
552 )
553 .unwrap();
555 554
556 let mut harness = Self { 555 let mut server = Command::new(env!("CARGO_BIN_EXE_git-collab-server"))
557 root, 556 .args(["--config", config_path.to_str().unwrap()])
558 repo_name: repo_name.to_string(), 557 .stdout(Stdio::piped())
559 work_repo, 558 .stderr(Stdio::piped())
560 server, 559 .spawn()
561 http_addr, 560 .expect("failed to start git-collab-server");
562 ssh_addr, 561
563 }; 562 match wait_until_ready(&mut server, http_addr, ssh_addr) {
564 harness.wait_until_ready(); 563 Ok(()) => {
565 harness 564 return Self {
565 root,
566 repo_name: repo_name.to_string(),
567 work_repo,
568 server,
569 http_addr,
570 ssh_addr,
571 };
572 }
573 Err(e) => {
574 last_error = format!("attempt {attempt}: {e}");
575 let _ = server.kill();
576 let _ = server.wait();
577 }
578 }
579 }
580
581 panic!("git-collab-server never became ready ({last_error})");
566 } 582 }
567 583
568 pub fn repo_name(&self) -> &str { 584 pub fn repo_name(&self) -> &str {
@@ -660,7 +676,11 @@ impl ServerHarness {
660 676
661 let deadline = Instant::now() + Duration::from_secs(60); 677 let deadline = Instant::now() + Duration::from_secs(60);
662 loop { 678 loop {
663 if child.try_wait().expect("failed to poll ssh process").is_some() { 679 if child
680 .try_wait()
681 .expect("failed to poll ssh process")
682 .is_some()
683 {
664 break; 684 break;
665 } 685 }
666 if Instant::now() >= deadline { 686 if Instant::now() >= deadline {
@@ -777,42 +797,73 @@ impl ServerHarness {
777 let head = String::from_utf8_lossy(&raw[..split]).to_string(); 797 let head = String::from_utf8_lossy(&raw[..split]).to_string();
778 (head, raw[split + 4..].to_vec()) 798 (head, raw[split + 4..].to_vec())
779 } 799 }
800 }
780 801
781 fn wait_until_ready(&mut self) { 802 /// Wait until both listeners accept connections. On failure the server is
782 let deadline = Instant::now() + Duration::from_secs(10); 803 /// reaped and its stderr folded into the error, so a flake says why it died
783 loop { 804 /// instead of just reporting an exit code.
784 if let Some(status) = self.server_status() { 805 fn wait_until_ready(
785 panic!("git-collab-server exited before becoming ready: {status}"); 806 server: &mut Child,
786 } 807 http_addr: SocketAddr,
787 808 ssh_addr: SocketAddr,
788 if let Ok(response) = TcpStream::connect(self.http_addr) { 809 ) -> Result<(), String> {
789 drop(response); 810 let deadline = Instant::now() + Duration::from_secs(10);
790 let response = self.get("/"); 811 loop {
791 if !response.status_line.is_empty() && TcpStream::connect(self.ssh_addr).is_ok() { 812 if let Ok(Some(status)) = server.try_wait() {
792 return; 813 return Err(format!(
793 } 814 "git-collab-server exited before becoming ready: exit status {:?}{}",
794 } 815 status.code(),
816 server_stderr(server)
817 ));
818 }
795 819
796 if Instant::now() >= deadline { 820 if probe_http_status_line(http_addr).is_some_and(|line| !line.is_empty())
797 panic!( 821 && TcpStream::connect(ssh_addr).is_ok()
798 "timed out waiting for git-collab-server on http {} / ssh {}", 822 {
799 self.http_addr, self.ssh_addr 823 return Ok(());
800 ); 824 }
801 }
802 825
803 thread::sleep(Duration::from_millis(50)); 826 if Instant::now() >= deadline {
827 let _ = server.kill();
828 let _ = server.wait();
829 return Err(format!(
830 "timed out waiting for git-collab-server on http {http_addr} / ssh {ssh_addr}{}",
831 server_stderr(server)
832 ));
804 } 833 }
834
835 thread::sleep(Duration::from_millis(50));
805 } 836 }
837 }
806 838
807 fn server_status(&mut self) -> Option<String> { 839 /// Drain the (already exited or killed) server's stderr for error messages.
808 self.server 840 fn server_stderr(server: &mut Child) -> String {
809 .try_wait() 841 match server.stderr.take() {
810 .ok() 842 Some(mut stderr) => {
811 .flatten() 843 let mut buf = String::new();
812 .map(|status| format!("exit status {:?}", status.code())) 844 let _ = stderr.read_to_string(&mut buf);
845 if buf.trim().is_empty() {
846 "\nserver stderr: <empty>".to_string()
847 } else {
848 format!("\nserver stderr:\n{buf}")
849 }
850 }
851 None => "\nserver stderr: <unavailable>".to_string(),
813 } 852 }
814 } 853 }
815 854
855 fn probe_http_status_line(addr: SocketAddr) -> Option<String> {
856 let mut stream = TcpStream::connect(addr).ok()?;
857 stream
858 .write_all(
859 format!("GET / HTTP/1.1\r\nHost: {addr}\r\nConnection: close\r\n\r\n").as_bytes(),
860 )
861 .ok()?;
862 let mut raw = String::new();
863 stream.read_to_string(&mut raw).ok()?;
864 Some(raw.lines().next().unwrap_or("").to_string())
865 }
866
816 impl Drop for ServerHarness { 867 impl Drop for ServerHarness {
817 fn drop(&mut self) { 868 fn drop(&mut self) {
818 let _ = self.server.kill(); 869 let _ = self.server.kill();
tests/release_server_test.rs
Old New
@@ -167,21 +167,107 @@ fn delete_removes_file_then_version() {
167 fn write_policy_gates_upload_and_delete_but_not_list() { 167 fn write_policy_gates_upload_and_delete_but_not_list() {
168 let harness = ServerHarness::new("release-policy"); 168 let harness = ServerHarness::new("release-policy");
169 harness.push_head(); 169 harness.push_head();
170
171 // Seed a release while the policy is still permissive, so the read-only
172 // assertions below have something real to observe. Without this, "list
173 // succeeds" and "delete was denied" would both be vacuously true against
174 // an empty releases dir.
175 let seeded = harness.ssh_exec_with_stdin(
176 "collab-release upload 'release-policy.git' 'v1' 'a.tar.gz'",
177 b"seed",
178 );
179 assert!(seeded.status.success(), "seed upload failed: {:?}", seeded);
180
170 harness.write_repo_server_policy( 181 harness.write_repo_server_policy(
171 "visibility = \"public\"\n[access]\nread = [\"*\"]\nwrite = []\n", 182 "visibility = \"public\"\n[access]\nread = [\"*\"]\nwrite = []\n",
172 ); 183 );
173 184
185 // Denied writes report "repository not found" - unknown and unauthorized
186 // deliberately share a reply so the error can't be used to probe which
187 // repos exist.
174 let upload = harness.ssh_exec_with_stdin( 188 let upload = harness.ssh_exec_with_stdin(
175 "collab-release upload 'release-policy.git' 'v1' 'a.tar.gz'", 189 "collab-release upload 'release-policy.git' 'v2' 'b.tar.gz'",
176 b"x", 190 b"x",
177 ); 191 );
178 assert!(!upload.status.success(), "upload must be denied"); 192 assert_ssh_error(&upload, "repository not found");
179 193
180 let delete = harness.ssh_exec("collab-release delete 'release-policy.git' 'v1'"); 194 let delete = harness.ssh_exec("collab-release delete 'release-policy.git' 'v1'");
181 assert!(!delete.status.success(), "delete must be denied"); 195 assert_ssh_error(&delete, "repository not found");
182 196
183 let list = harness.ssh_exec("collab-release list 'release-policy.git'"); 197 let list = harness.ssh_exec("collab-release list 'release-policy.git'");
184 assert!(list.status.success(), "list must be allowed for readers"); 198 assert!(list.status.success(), "list must be allowed for readers");
199
200 // The seeded release is still listed: proof the denied delete didn't run.
201 let index: serde_json::Value = serde_json::from_str(&stdout(&list)).unwrap();
202 let versions = index["versions"].as_array().unwrap();
203 assert_eq!(versions.len(), 1, "seeded release must survive: {}", index);
204 assert_eq!(versions[0]["version"], "v1");
205
206 // And on disk: the seed is intact, the denied upload left nothing behind.
207 let releases = harness
208 .repos_dir()
209 .join("release-policy.git/collab/releases");
210 assert_eq!(
211 std::fs::read(releases.join("v1/a.tar.gz")).unwrap(),
212 b"seed"
213 );
214 assert!(!releases.join("v2/b.tar.gz").exists());
215 assert!(!releases.join("v2").exists());
216 }
217
218 #[test]
219 fn git_push_and_clone_over_ssh() {
220 // Exercises exec_request's git branch, stdin forwarding through data(),
221 // and channel_eof against a real git client - the path the release
222 // dispatch had to be threaded through without regressing.
223 let harness = ServerHarness::new("release-git");
224 let ssh_command = harness.ssh_command_string();
225
226 harness.work_repo().git(&[
227 "-c",
228 &format!("core.sshCommand={}", ssh_command),
229 "push",
230 &harness.repo_ssh_url(),
231 "main",
232 ]);
233
234 let expected = harness.work_repo().git(&["rev-parse", "HEAD"]);
235 let expected = expected.trim();
236
237 let clone_dir = tempfile::TempDir::new().unwrap();
238 let dest = clone_dir.path().join("clone");
239 // Reuse the work repo's isolated HOME so the developer's real git config
240 // can't influence the clone.
241 let mut clone_cmd = std::process::Command::new("git");
242 harness.work_repo().apply_env(&mut clone_cmd);
243 let output = clone_cmd
244 .args([
245 "-c",
246 &format!("core.sshCommand={}", ssh_command),
247 "clone",
248 &harness.repo_ssh_url(),
249 dest.to_str().unwrap(),
250 ])
251 .output()
252 .expect("failed to run git clone");
253 assert!(
254 output.status.success(),
255 "clone over ssh failed: {}",
256 String::from_utf8_lossy(&output.stderr)
257 );
258
259 let mut head_cmd = std::process::Command::new("git");
260 harness.work_repo().apply_env(&mut head_cmd);
261 let cloned_head = head_cmd
262 .args(["rev-parse", "HEAD"])
263 .current_dir(&dest)
264 .output()
265 .expect("failed to run git rev-parse in clone");
266 assert_eq!(
267 String::from_utf8_lossy(&cloned_head.stdout).trim(),
268 expected,
269 "cloned repo is not at the pushed commit"
270 );
185 } 271 }
186 272
187 #[test] 273 #[test]
@@ -219,3 +305,72 @@ fn invalid_names_and_unknown_repo_rejected() {
219 harness.ssh_exec_with_stdin("collab-release upload 'nope.git' 'v1' 'a.tar.gz'", b"x"); 305 harness.ssh_exec_with_stdin("collab-release upload 'nope.git' 'v1' 'a.tar.gz'", b"x");
220 assert!(!unknown.status.success()); 306 assert!(!unknown.status.success());
221 } 307 }
308
309 #[test]
310 fn http_releases_page_and_download() {
311 let harness = ServerHarness::new("release-http");
312 harness.push_head();
313
314 let content: Vec<u8> = (0u32..600).flat_map(|i| i.to_le_bytes()).collect(); // binary body
315 harness.ssh_exec_with_stdin(
316 "collab-release upload 'release-http.git' 'v2.0.0' 'app.tar.gz'",
317 &content,
318 );
319
320 let page = harness.get_ok("/release-http/releases");
321 assert!(page.body.contains("v2.0.0"));
322 assert!(page.body.contains("app.tar.gz"));
323 assert!(page.body.contains("/release-http/releases/v2.0.0/app.tar.gz"));
324
325 let (head, body) = harness.get_bytes("/release-http/releases/v2.0.0/app.tar.gz");
326 assert!(head.contains("200"), "download failed: {}", head);
327 assert!(head.to_lowercase().contains("application/octet-stream"));
328 assert!(head
329 .to_lowercase()
330 .contains(&format!("content-length: {}", content.len())));
331 assert_eq!(body, content);
332
333 // checksum companion is downloadable as text
334 let (sha_head, sha_body) =
335 harness.get_bytes("/release-http/releases/v2.0.0/app.tar.gz.sha256");
336 assert!(sha_head.contains("200"));
337 assert!(String::from_utf8(sha_body).unwrap().contains("app.tar.gz"));
338 }
339
340 #[test]
341 fn http_release_download_404s() {
342 let harness = ServerHarness::new("release-http-404");
343 harness.push_head();
344
345 let missing = harness.get("/release-http-404/releases/v9/none.tar.gz");
346 assert!(missing.status_line.contains("404"));
347
348 let traversal = harness.get("/release-http-404/releases/v9/..%2f..%2fconfig");
349 assert!(!traversal.status_line.contains("200"));
350
351 let page = harness.get_ok("/release-http-404/releases");
352 assert!(page.body.contains("No releases"));
353 }
354
355 #[test]
356 fn http_releases_respect_repo_policy() {
357 let harness = ServerHarness::new("release-http-private");
358 harness.push_head();
359 harness.ssh_exec_with_stdin(
360 "collab-release upload 'release-http-private.git' 'v1' 'a.tar.gz'",
361 b"secret",
362 );
363 harness.write_repo_server_policy(
364 "visibility = \"private\"\n[ui]\nanonymous = false\n[http]\nanonymous_clone = false\n[access]\nread = [\"*\"]\nwrite = [\"*\"]\n",
365 );
366
367 let page = harness.get("/release-http-private/releases");
368 assert!(page.status_line.contains("404"));
369
370 let (head, _) = harness.get_bytes("/release-http-private/releases/v1/a.tar.gz");
371 assert!(
372 head.contains("404"),
373 "private artifact must not be served: {}",
374 head
375 );
376 }