max / makenotwork
- Co-Authored-By
- Claude Opus 5 (1M context) <noreply@anthropic.com>
40 files changed,
+1247 insertions,
-262 deletions
| @@ -1186,6 +1186,10 @@ | |||
| 1186 | 1186 | "polyval", | |
| 1187 | 1187 | ] | |
| 1188 | 1188 | ||
| 1189 | + | [[package]] | |
| 1190 | + | name = "git-command" | |
| 1191 | + | version = "0.1.0" | |
| 1192 | + | ||
| 1189 | 1193 | [[package]] | |
| 1190 | 1194 | name = "group" | |
| 1191 | 1195 | version = "0.14.0" | |
| @@ -1932,6 +1936,7 @@ | |||
| 1932 | 1936 | "anyhow", | |
| 1933 | 1937 | "bytes", | |
| 1934 | 1938 | "crossterm", | |
| 1939 | + | "git-command", | |
| 1935 | 1940 | "hex", | |
| 1936 | 1941 | "hmac", | |
| 1937 | 1942 | "makeover", |
| @@ -29,6 +29,10 @@ | |||
| 29 | 29 | tracing-subscriber = { version = "0.3", features = ["env-filter"] } | |
| 30 | 30 | anyhow = "1" | |
| 31 | 31 | bytes = "1" | |
| 32 | + | ||
| 33 | + | # The git-over-SSH command grammar, shared with the server's git_ssh door so | |
| 34 | + | # the two SSH doors cannot drift apart the way their hand-written parsers did. | |
| 35 | + | git-command = { path = "../shared/git-command" } | |
| 32 | 36 | hmac = "0.13.0" | |
| 33 | 37 | sha2 = "0.11.0" | |
| 34 | 38 | hex = "0.4.3" |
| @@ -3251,6 +3251,10 @@ | |||
| 3251 | 3251 | "stable_deref_trait", | |
| 3252 | 3252 | ] | |
| 3253 | 3253 | ||
| 3254 | + | [[package]] | |
| 3255 | + | name = "git-command" | |
| 3256 | + | version = "0.1.0" | |
| 3257 | + | ||
| 3254 | 3258 | [[package]] | |
| 3255 | 3259 | name = "gix" | |
| 3256 | 3260 | version = "0.86.0" | |
| @@ -5226,6 +5230,7 @@ | |||
| 5226 | 5230 | "email_address", | |
| 5227 | 5231 | "flate2", | |
| 5228 | 5232 | "fs2", | |
| 5233 | + | "git-command", | |
| 5229 | 5234 | "gix", | |
| 5230 | 5235 | "goblin 0.10.7", | |
| 5231 | 5236 | "governor", | |
| @@ -10682,14 +10687,6 @@ | |||
| 10682 | 10687 | "pkg-config", | |
| 10683 | 10688 | ] | |
| 10684 | 10689 | ||
| 10685 | - | [[patch.unused]] | |
| 10686 | - | name = "quasi-store" | |
| 10687 | - | version = "0.1.0" | |
| 10688 | - | ||
| 10689 | - | [[patch.unused]] | |
| 10690 | - | name = "quasi-tauri" | |
| 10691 | - | version = "0.1.0" | |
| 10692 | - | ||
| 10693 | 10690 | [[patch.unused]] | |
| 10694 | 10691 | name = "synckit-client" | |
| 10695 | 10692 | version = "0.8.0" | |
| @@ -10709,3 +10706,11 @@ | |||
| 10709 | 10706 | [[patch.unused]] | |
| 10710 | 10707 | name = "painhours" | |
| 10711 | 10708 | version = "0.1.0" | |
| 10709 | + | ||
| 10710 | + | [[patch.unused]] | |
| 10711 | + | name = "quasi-store" | |
| 10712 | + | version = "0.1.0" | |
| 10713 | + | ||
| 10714 | + | [[patch.unused]] | |
| 10715 | + | name = "quasi-tauri" | |
| 10716 | + | version = "0.1.0" |
| @@ -147,6 +147,10 @@ | |||
| 147 | 147 | # Tag standard | |
| 148 | 148 | tagtree = { path = "../shared/tagtree" } | |
| 149 | 149 | ||
| 150 | + | # The git-over-SSH command grammar, shared with mnw-cli so the two SSH doors | |
| 151 | + | # cannot drift apart the way their hand-written parsers did. | |
| 152 | + | git-command = { path = "../shared/git-command" } | |
| 153 | + | ||
| 150 | 154 | # Shared theme palette + the bundled theme set (Tier 0 creator theming). | |
| 151 | 155 | makeover = "2.5.0" | |
| 152 | 156 |
| @@ -4,6 +4,7 @@ | |||
| 4 | 4 | //! `command=` prefix in authorized_keys. Handles git push/pull access control | |
| 5 | 5 | //! and interactive management commands (repo list, key management, etc.). | |
| 6 | 6 | ||
| 7 | + | use git_command::{Operation, Request}; | |
| 7 | 8 | use sqlx::PgPool; | |
| 8 | 9 | use std::fmt::Write as _; | |
| 9 | 10 | ||
| @@ -31,23 +32,11 @@ | |||
| 31 | 32 | } | |
| 32 | 33 | ||
| 33 | 34 | // ── Git operations ── | |
| 34 | - | ||
| 35 | - | #[derive(Debug)] | |
| 36 | - | enum GitOperation { | |
| 37 | - | UploadPack, | |
| 38 | - | ReceivePack, | |
| 39 | - | Archive, | |
| 40 | - | } | |
| 41 | - | ||
| 42 | - | impl GitOperation { | |
| 43 | - | fn command(&self) -> &'static str { | |
| 44 | - | match self { | |
| 45 | - | Self::UploadPack => "git-upload-pack", | |
| 46 | - | Self::ReceivePack => "git-receive-pack", | |
| 47 | - | Self::Archive => "git-upload-archive", | |
| 48 | - | } | |
| 49 | - | } | |
| 50 | - | } | |
| 35 | + | // | |
| 36 | + | // The command grammar itself lives in `git-command`, shared with mnw-cli's | |
| 37 | + | // russh door. It used to be parsed here and again there, and the two parsers | |
| 38 | + | // disagreed on 51 of 5,424 measured command lines. See that crate's module docs | |
| 39 | + | // for the four divergences and how each was settled. | |
| 51 | 40 | ||
| 52 | 41 | /// Authenticate and dispatch an SSH git-auth invocation. | |
| 53 | 42 | /// | |
| @@ -100,13 +89,20 @@ | |||
| 100 | 89 | user_id: UserId, | |
| 101 | 90 | original_cmd: &str, | |
| 102 | 91 | ) -> anyhow::Result<()> { | |
| 103 | - | let (operation, repo_path) = parse_ssh_command(original_cmd)?; | |
| 104 | - | let (owner, repo_name) = parse_repo_path(&repo_path)?; | |
| 92 | + | // `git_command::parse` guarantees path safety: both segments are single, | |
| 93 | + | // non-empty, non-traversing components, so nothing below can leave the git | |
| 94 | + | // root. What it deliberately does not decide is identity policy, which is | |
| 95 | + | // this deployment's and stays here. | |
| 96 | + | let request: Request<'_> = | |
| 97 | + | git_command::parse(original_cmd).map_err(|e| anyhow::anyhow!("{e}"))?; | |
| 98 | + | let (operation, owner, repo_name) = (request.operation, request.owner, request.repo); | |
| 105 | 99 | ||
| 106 | - | // Validate the SSH-supplied owner and repo name before any DB lookup or | |
| 107 | - | // shell reconstruction. `parse_repo_path` is a path-shape check, not a | |
| 108 | - | // syntax check, without this, a malformed name could reach the DB layer | |
| 109 | - | // or end up embedded in the `git-shell -c` argument below. | |
| 100 | + | // `Username::new` is the identity rule (3-50 chars, alphanumeric and | |
| 101 | + | // underscore) and is stricter than the path-safety floor the parser | |
| 102 | + | // enforces. `validate_git_repo_name` is the product's repo-name policy, the | |
| 103 | + | // same one the web API applies when a repo is created there; it currently | |
| 104 | + | // matches `git_command::valid_segment` exactly, and it is kept because the | |
| 105 | + | // two answer different questions and are free to diverge. | |
| 110 | 106 | let owner_username = | |
| 111 | 107 | Username::new(owner).map_err(|_| anyhow::anyhow!("repository not found"))?; | |
| 112 | 108 | validate_git_repo_name(repo_name).map_err(|_| anyhow::anyhow!("repository not found"))?; | |
| @@ -120,7 +116,7 @@ | |||
| 120 | 116 | Some(repo) => repo, | |
| 121 | 117 | None => { | |
| 122 | 118 | // Auto-create on push if the authenticated user owns the namespace. | |
| 123 | - | if !matches!(operation, GitOperation::ReceivePack) || user_id != owner_user.id { | |
| 119 | + | if operation != Operation::ReceivePack || user_id != owner_user.id { | |
| 124 | 120 | anyhow::bail!("repository not found"); | |
| 125 | 121 | } | |
| 126 | 122 | ||
| @@ -132,7 +128,7 @@ | |||
| 132 | 128 | // Permission check, owner always has full access, collaborators checked via DB | |
| 133 | 129 | let is_owner = user_id == owner_user.id; | |
| 134 | 130 | match operation { | |
| 135 | - | GitOperation::ReceivePack => { | |
| 131 | + | Operation::ReceivePack => { | |
| 136 | 132 | if !is_owner { | |
| 137 | 133 | let can_push = db::repo_collaborators::can_user_push(pool, repo.id, user_id) | |
| 138 | 134 | .await | |
| @@ -159,7 +155,7 @@ | |||
| 159 | 155 | ||
| 160 | 156 | ensure_bare_repo_on_disk(&git_repos_root(), owner_username.as_ref(), repo_name)?; | |
| 161 | 157 | } | |
| 162 | - | GitOperation::UploadPack | GitOperation::Archive => { | |
| 158 | + | Operation::UploadPack | Operation::UploadArchive => { | |
| 163 | 159 | if repo.visibility == db::Visibility::Private && !is_owner { | |
| 164 | 160 | let is_collab = db::repo_collaborators::is_collaborator(pool, repo.id, user_id) | |
| 165 | 161 | .await | |
| @@ -177,19 +173,13 @@ | |||
| 177 | 173 | } | |
| 178 | 174 | } | |
| 179 | 175 | ||
| 180 | - | // Authorized, exec git-shell with a sanitized command reconstructed | |
| 181 | - | // from validated components (prevents argument injection via the original | |
| 182 | - | // command). Use `owner_username` (the `Username`-validated value), not the | |
| 183 | - | // raw `owner` &str: `Username::new` preserves the string but constrains the | |
| 184 | - | // charset, so the value flowing into the `git-shell -c` argument stays | |
| 185 | - | // load-bearing on the validated type even if `parse_repo_path` ever loosens. | |
| 186 | - | let sanitized_cmd = format!( | |
| 187 | - | "{} '/{}/{}.git'", | |
| 188 | - | operation.command(), | |
| 189 | - | owner_username.as_ref(), | |
| 190 | - | repo_name | |
| 191 | - | ); | |
| 192 | - | run_git_shell(&sanitized_cmd).await | |
| 176 | + | // Authorized. Exec git-shell with a command rebuilt from validated | |
| 177 | + | // components rather than with the line the client sent, which is what keeps | |
| 178 | + | // argument injection out. The rebuild lives in `git_command` so there is one | |
| 179 | + | // format string on this path and mnw-cli's, and the segments going into it | |
| 180 | + | // cannot carry a quote or a separator: `valid_segment` is a whitelist and | |
| 181 | + | // ran before this request existed. | |
| 182 | + | run_git_shell(&request.shell_command()).await | |
| 193 | 183 | } | |
| 194 | 184 | ||
| 195 | 185 | /// Create the bare repo a push is about to write into, if it is not there. | |
| @@ -251,50 +241,6 @@ | |||
| 251 | 241 | ) | |
| 252 | 242 | } | |
| 253 | 243 | ||
| 254 | - | fn parse_ssh_command(cmd: &str) -> anyhow::Result<(GitOperation, String)> { | |
| 255 | - | let parts: Vec<&str> = cmd.splitn(2, ' ').collect(); | |
| 256 | - | if parts.len() != 2 { | |
| 257 | - | anyhow::bail!("invalid git command"); | |
| 258 | - | } | |
| 259 | - | ||
| 260 | - | let operation = match parts[0] { | |
| 261 | - | "git-upload-pack" => GitOperation::UploadPack, | |
| 262 | - | "git-receive-pack" => GitOperation::ReceivePack, | |
| 263 | - | "git-upload-archive" => GitOperation::Archive, | |
| 264 | - | _ => anyhow::bail!("unsupported git command: {}", parts[0]), | |
| 265 | - | }; | |
| 266 | - | ||
| 267 | - | let repo_path = parts[1].trim_matches('\'').trim_matches('"'); | |
| 268 | - | Ok((operation, repo_path.to_string())) | |
| 269 | - | } | |
| 270 | - | ||
| 271 | - | fn parse_repo_path(path: &str) -> anyhow::Result<(&str, &str)> { | |
| 272 | - | let path = path.trim_start_matches('/'); | |
| 273 | - | let (owner, rest) = path | |
| 274 | - | .split_once('/') | |
| 275 | - | .ok_or_else(|| anyhow::anyhow!("invalid repository path: missing owner or repo"))?; | |
| 276 | - | ||
| 277 | - | if owner.contains("..") || rest.contains("..") { | |
| 278 | - | anyhow::bail!("invalid repository path: path traversal not allowed"); | |
| 279 | - | } | |
| 280 | - | ||
| 281 | - | // Reject lone-dot segments, `parse_repo_path` is the gate before the | |
| 282 | - | // `format!("{op} '/{owner}/{repo_name}.git'")` that flows into `git-shell`. | |
| 283 | - | // `validate_git_repo_name` below would also catch most of these, but the | |
| 284 | - | // belt-and-braces rejection here keeps the dispatch path itself strict. | |
| 285 | - | if owner == "." || rest.split('/').any(|seg| seg == "." || seg == "..") { | |
| 286 | - | anyhow::bail!("invalid repository path: lone-dot segment not allowed"); | |
| 287 | - | } | |
| 288 | - | ||
| 289 | - | let repo_name = rest.strip_suffix(".git").unwrap_or(rest); | |
| 290 | - | ||
| 291 | - | if owner.is_empty() || repo_name.is_empty() { | |
| 292 | - | anyhow::bail!("invalid repository path: empty owner or repo name"); | |
| 293 | - | } | |
| 294 | - | ||
| 295 | - | Ok((owner, repo_name)) | |
| 296 | - | } | |
| 297 | - | ||
| 298 | 244 | /// Run git-shell as a child with inherited stdio (the ssh channel's fds) under a | |
| 299 | 245 | /// runaway-backstop timeout, then exit this process with the child's status. | |
| 300 | 246 | /// | |
| @@ -428,7 +374,7 @@ | |||
| 428 | 374 | mod tests { | |
| 429 | 375 | use super::*; | |
| 430 | 376 | ||
| 431 | - | // ── parse_ssh_command ── | |
| 377 | + | // ── the git-command door ── | |
| 432 | 378 | ||
| 433 | 379 | // The push path's half of repo creation. Registering the row was never the | |
| 434 | 380 | // part that broke; this is. | |
| @@ -450,84 +396,38 @@ | |||
| 450 | 396 | assert!(gix::open(&repo_dir).is_ok()); | |
| 451 | 397 | } | |
| 452 | 398 | ||
| 399 | + | // The grammar's own tests live in `git-command`, which owns the parser. | |
| 400 | + | // What is worth asserting here is that this door reaches it and keeps the | |
| 401 | + | // guarantee the rest of the function leans on: a request that parses names | |
| 402 | + | // one owner and one repo, and neither can leave the git root. | |
| 403 | + | ||
| 453 | 404 | #[test] | |
| 454 | - | fn parse_upload_pack() { | |
| 455 | - | let (op, path) = parse_ssh_command("git-upload-pack '/user/repo.git'").unwrap(); | |
| 456 | - | assert!(matches!(op, GitOperation::UploadPack)); | |
| 457 | - | assert_eq!(path, "/user/repo.git"); | |
| 405 | + | fn the_door_parses_what_a_client_sends() { | |
| 406 | + | let r = git_command::parse("git-receive-pack '/user/repo.git'").unwrap(); | |
| 407 | + | assert_eq!(r.operation, Operation::ReceivePack); | |
| 408 | + | assert_eq!((r.owner, r.repo), ("user", "repo")); | |
| 458 | 409 | } | |
| 459 | 410 | ||
| 460 | 411 | #[test] | |
| 461 | - | fn parse_receive_pack() { | |
| 462 | - | let (op, path) = parse_ssh_command("git-receive-pack '/user/repo.git'").unwrap(); | |
| 463 | - | assert!(matches!(op, GitOperation::ReceivePack)); | |
| 464 | - | assert_eq!(path, "/user/repo.git"); | |
| 412 | + | fn the_shell_argument_is_rebuilt_from_validated_parts() { | |
| 413 | + | let r = git_command::parse("git-upload-pack '/user/repo.git'").unwrap(); | |
| 414 | + | assert_eq!(r.shell_command(), "git-upload-pack '/user/repo.git'"); | |
| 465 | 415 | } | |
| 466 | 416 | ||
| 467 | 417 | #[test] | |
| 468 | - | fn parse_upload_archive() { | |
| 469 | - | let (op, path) = parse_ssh_command("git-upload-archive '/user/repo.git'").unwrap(); | |
| 470 | - | assert!(matches!(op, GitOperation::Archive)); | |
| 471 | - | assert_eq!(path, "/user/repo.git"); | |
| 418 | + | fn a_traversing_path_never_reaches_the_db_lookup() { | |
| 419 | + | for cmd in [ | |
| 420 | + | "git-upload-pack '/../etc/passwd'", | |
| 421 | + | "git-upload-pack '/user/../../etc'", | |
| 422 | + | "git-receive-pack '/user/.hidden.git'", | |
| 423 | + | "git-upload-pack '/user/a/b.git'", | |
| 424 | + | ] { | |
| 425 | + | assert!(git_command::parse(cmd).is_err(), "{cmd}"); | |
| 426 | + | } | |
| 472 | 427 | } | |
| 473 | 428 | ||
| 474 | 429 | #[test] | |
| 475 | - | fn parse_ssh_command_double_quotes() { | |
| 476 | - | let (_, path) = parse_ssh_command(r#"git-upload-pack "/user/repo.git""#).unwrap(); | |
| 477 | - | assert_eq!(path, "/user/repo.git"); | |
| 478 | - | } | |
| 479 | - | ||
| 480 | - | #[test] | |
| 481 | - | fn parse_ssh_command_unsupported() { | |
| 482 | - | assert!(parse_ssh_command("git-foo '/user/repo.git'").is_err()); | |
| 483 | - | } | |
| 484 | - | ||
| 485 | - | #[test] | |
| 486 | - | fn parse_ssh_command_no_space() { | |
| 487 | - | assert!(parse_ssh_command("git-upload-pack").is_err()); | |
| 488 | - | } | |
| 489 | - | ||
| 490 | - | // ── parse_repo_path ── | |
| 491 | - | ||
| 492 | - | #[test] | |
| 493 | - | fn parse_valid_repo_path() { | |
| 494 | - | let (owner, name) = parse_repo_path("/alice/myrepo.git").unwrap(); | |
| 495 | - | assert_eq!(owner, "alice"); | |
| 496 | - | assert_eq!(name, "myrepo"); | |
| 497 | - | } | |
| 498 | - | ||
| 499 | - | #[test] | |
| 500 | - | fn parse_repo_path_no_git_suffix() { | |
| 501 | - | let (owner, name) = parse_repo_path("/bob/project").unwrap(); | |
| 502 | - | assert_eq!(owner, "bob"); | |
| 503 | - | assert_eq!(name, "project"); | |
| 504 | - | } | |
| 505 | - | ||
| 506 | - | #[test] | |
| 507 | - | fn parse_repo_path_no_leading_slash() { | |
| 508 | - | let (owner, name) = parse_repo_path("carol/stuff.git").unwrap(); | |
| 509 | - | assert_eq!(owner, "carol"); | |
| 510 | - | assert_eq!(name, "stuff"); | |
| 511 | - | } | |
| 512 | - | ||
| 513 | - | #[test] | |
| 514 | - | fn parse_repo_path_traversal_rejected() { | |
| 515 | - | assert!(parse_repo_path("../evil/repo").is_err()); | |
| 516 | - | assert!(parse_repo_path("user/../repo").is_err()); | |
| 517 | - | } | |
| 518 | - | ||
| 519 | - | #[test] | |
| 520 | - | fn parse_repo_path_missing_repo() { | |
| 521 | - | assert!(parse_repo_path("/onlyowner").is_err()); | |
| 522 | - | } | |
| 523 | - | ||
| 524 | - | #[test] | |
| 525 | - | fn parse_repo_path_empty_owner() { | |
| 526 | - | assert!(parse_repo_path("//repo").is_err()); | |
| 527 | - | } | |
| 528 | - | ||
| 529 | - | #[test] | |
| 530 | - | fn parse_repo_path_bare_git_suffix_only() { | |
| 531 | - | assert!(parse_repo_path("/owner/.git").is_err()); | |
| 430 | + | fn management_verbs_are_not_this_grammar() { | |
| 431 | + | assert!(git_command::parse("repo list").is_err()); | |
| 532 | 432 | } | |
| 533 | 433 | } |
| @@ -10,51 +10,21 @@ | |||
| 10 | 10 | use tokio::io::AsyncReadExt; | |
| 11 | 11 | use tokio::process::{Child, Command}; | |
| 12 | 12 | ||
| 13 | - | /// Parse a git exec command into (operation, raw_path). | |
| 13 | + | /// Parse a git exec command line. | |
| 14 | 14 | /// | |
| 15 | - | /// Git clients send commands like: | |
| 16 | - | /// `git-upload-pack '/max/repo.git'` | |
| 17 | - | /// `git-receive-pack 'max/repo.git'` | |
| 15 | + | /// The grammar lives in `git-command`, shared with the server's `git_ssh` door. | |
| 16 | + | /// It used to be parsed here and again there, and the two hand-written parsers | |
| 17 | + | /// disagreed on 51 of 5,424 measured command lines: leading whitespace, | |
| 18 | + | /// unbalanced quotes, repeated leading slashes, and whether `..` was tested | |
| 19 | + | /// before or after the `.git` suffix came off. The last of those was this | |
| 20 | + | /// module's, and it was the one with teeth — `/max/repo..git` parsed here as | |
| 21 | + | /// repo `repo.`, safe only because a charset guard further in happened to | |
| 22 | + | /// catch it. | |
| 18 | 23 | /// | |
| 19 | - | /// Returns `None` for non-git commands. | |
| 20 | - | pub(crate) fn parse_git_command(cmd: &str) -> Option<(&str, &str)> { | |
| 21 | - | let (operation, rest) = cmd.split_once(' ')?; | |
| 22 | - | ||
| 23 | - | match operation { | |
| 24 | - | "git-upload-pack" | "git-receive-pack" | "git-upload-archive" => {} | |
| 25 | - | _ => return None, | |
| 26 | - | } | |
| 27 | - | ||
| 28 | - | // Strip surrounding quotes (single or double) | |
| 29 | - | let path = rest.trim(); | |
| 30 | - | let path = path | |
| 31 | - | .strip_prefix('\'') | |
| 32 | - | .and_then(|s| s.strip_suffix('\'')) | |
| 33 | - | .or_else(|| path.strip_prefix('"').and_then(|s| s.strip_suffix('"'))) | |
| 34 | - | .unwrap_or(path); | |
| 35 | - | ||
| 36 | - | Some((operation, path)) | |
| 37 | - | } | |
| 38 | - | ||
| 39 | - | /// Parse a repo path like "max/repo.git" or "/max/repo" into (owner, repo_name). | |
| 40 | - | /// | |
| 41 | - | /// Strips leading `/` and trailing `.git`. | |
| 42 | - | pub(crate) fn parse_repo_path(path: &str) -> Option<(&str, &str)> { | |
| 43 | - | let path = path.strip_prefix('/').unwrap_or(path); | |
| 44 | - | let (owner, repo_name) = path.split_once('/')?; | |
| 45 | - | ||
| 46 | - | if owner.is_empty() || owner.contains("..") { | |
| 47 | - | return None; | |
| 48 | - | } | |
| 49 | - | ||
| 50 | - | // Strip trailing .git | |
| 51 | - | let repo_name = repo_name.strip_suffix(".git").unwrap_or(repo_name); | |
| 52 | - | ||
| 53 | - | if repo_name.is_empty() || repo_name.contains("..") || repo_name.contains('/') { | |
| 54 | - | return None; | |
| 55 | - | } | |
| 56 | - | ||
| 57 | - | Some((owner, repo_name)) | |
| 24 | + | /// Returns `None` for anything that is not a well-formed, path-safe git | |
| 25 | + | /// request, which is every case this door refuses. | |
| 26 | + | pub(crate) fn parse_command(cmd: &str) -> Option<git_command::Request<'_>> { | |
| 27 | + | git_command::parse(cmd).ok() | |
| 58 | 28 | } | |
| 59 | 29 | ||
| 60 | 30 | /// Spawn a git subprocess and wire its I/O through the SSH channel. | |
| @@ -264,31 +234,47 @@ | |||
| 264 | 234 | mod tests { | |
| 265 | 235 | use super::*; | |
| 266 | 236 | ||
| 237 | + | // The grammar's tests live in `git-command`, which owns the parser. What | |
| 238 | + | // this door owes is proof that it reaches it and refuses what it should. | |
| 239 | + | ||
| 267 | 240 | #[test] | |
| 268 | - | fn parse_git_upload_pack() { | |
| 269 | - | let (op, path) = parse_git_command("git-upload-pack '/max/repo.git'").unwrap(); | |
| 270 | - | assert_eq!(op, "git-upload-pack"); | |
| 271 | - | assert_eq!(path, "/max/repo.git"); | |
| 241 | + | fn the_three_verbs_a_client_sends() { | |
| 242 | + | let r = parse_command("git-upload-pack '/max/repo.git'").unwrap(); | |
| 243 | + | assert_eq!(r.operation.command(), "git-upload-pack"); | |
| 244 | + | assert_eq!((r.owner, r.repo), ("max", "repo")); | |
| 245 | + | ||
| 246 | + | let r = parse_command("git-receive-pack max/repo.git").unwrap(); | |
| 247 | + | assert_eq!(r.operation.command(), "git-receive-pack"); | |
| 248 | + | ||
| 249 | + | let r = parse_command("git-upload-archive \"/max/repo.git\"").unwrap(); | |
| 250 | + | assert_eq!(r.operation.command(), "git-upload-archive"); | |
| 272 | 251 | } | |
| 273 | 252 | ||
| 274 | 253 | #[test] | |
| 275 | - | fn parse_git_receive_pack_no_quotes() { | |
| 276 | - | let (op, path) = parse_git_command("git-receive-pack max/repo.git").unwrap(); | |
| 277 | - | assert_eq!(op, "git-receive-pack"); | |
| 278 | - | assert_eq!(path, "max/repo.git"); | |
| 254 | + | fn non_git_commands_are_refused() { | |
| 255 | + | assert!(parse_command("ls -la").is_none()); | |
| 256 | + | assert!(parse_command("scp -t /tmp/file").is_none()); | |
| 279 | 257 | } | |
| 280 | 258 | ||
| 281 | 259 | #[test] | |
| 282 | - | fn parse_git_upload_archive_double_quotes() { | |
| 283 | - | let (op, path) = parse_git_command("git-upload-archive \"/max/repo.git\"").unwrap(); | |
| 284 | - | assert_eq!(op, "git-upload-archive"); | |
| 285 | - | assert_eq!(path, "/max/repo.git"); | |
| 260 | + | fn path_unsafe_requests_are_refused() { | |
| 261 | + | for cmd in [ | |
| 262 | + | "git-upload-pack ../evil/repo.git", | |
| 263 | + | "git-upload-pack max/../../etc.git", | |
| 264 | + | "git-upload-pack max/sub/repo.git", | |
| 265 | + | "git-upload-pack max/.git", | |
| 266 | + | "git-upload-pack max/", | |
| 267 | + | "git-upload-pack /", | |
| 268 | + | ] { | |
| 269 | + | assert!(parse_command(cmd).is_none(), "{cmd}"); | |
| 270 | + | } | |
| 286 | 271 | } | |
| 287 | 272 | ||
| 273 | + | /// The divergence this door owned. `.git` used to come off before `..` was | |
| 274 | + | /// tested, so this parsed as repo `repo.`. | |
| 288 | 275 | #[test] | |
| 289 | - | fn parse_non_git_command() { | |
| 290 | - | assert!(parse_git_command("ls -la").is_none()); | |
| 291 | - | assert!(parse_git_command("scp -t /tmp/file").is_none()); | |
| 276 | + | fn dotdot_is_tested_before_the_git_suffix() { | |
| 277 | + | assert!(parse_command("git-upload-pack /max/repo..git").is_none()); | |
| 292 | 278 | } | |
| 293 | 279 | ||
| 294 | 280 | /// Locks the derivation to the same vector the server's `repo_hmac` | |
| @@ -329,44 +315,4 @@ | |||
| 329 | 315 | assert!(!body.contains("__HMAC__")); | |
| 330 | 316 | assert!(body.contains(&repo_hmac(token, "max", "repo"))); | |
| 331 | 317 | } | |
| 332 | - | ||
| 333 | - | #[test] | |
| 334 | - | fn parse_repo_path_basic() { | |
| 335 | - | let (owner, repo) = parse_repo_path("max/repo.git").unwrap(); | |
| 336 | - | assert_eq!(owner, "max"); | |
| 337 | - | assert_eq!(repo, "repo"); | |
| 338 | - | } | |
| 339 | - | ||
| 340 | - | #[test] | |
| 341 | - | fn parse_repo_path_with_leading_slash() { | |
| 342 | - | let (owner, repo) = parse_repo_path("/max/myproject.git").unwrap(); | |
| 343 | - | assert_eq!(owner, "max"); | |
| 344 | - | assert_eq!(repo, "myproject"); | |
| 345 | - | } | |
| 346 | - | ||
| 347 | - | #[test] | |
| 348 | - | fn parse_repo_path_no_git_suffix() { | |
| 349 | - | let (owner, repo) = parse_repo_path("max/repo").unwrap(); | |
| 350 | - | assert_eq!(owner, "max"); | |
| 351 | - | assert_eq!(repo, "repo"); | |
| 352 | - | } | |
| 353 | - | ||
| 354 | - | #[test] | |
| 355 | - | fn parse_repo_path_rejects_traversal() { | |
| 356 | - | assert!(parse_repo_path("../evil/repo.git").is_none()); | |
| 357 | - | assert!(parse_repo_path("max/../../etc.git").is_none()); | |
| 358 | - | } | |
| 359 | - | ||
| 360 | - | #[test] | |
| 361 | - | fn parse_repo_path_rejects_empty() { | |
| 362 | - | assert!(parse_repo_path("").is_none()); | |
| 363 | - | assert!(parse_repo_path("/").is_none()); | |
| 364 | - | assert!(parse_repo_path("max/").is_none()); | |
| 365 | - | assert!(parse_repo_path("max/.git").is_none()); | |
| 366 | - | } | |
| 367 | - | ||
| 368 | - | #[test] | |
| 369 | - | fn parse_repo_path_rejects_nested() { | |
| 370 | - | assert!(parse_repo_path("max/sub/repo.git").is_none()); | |
| 371 | - | } | |
| 372 | 318 | } |
| @@ -403,9 +403,10 @@ | |||
| 403 | 403 | }; | |
| 404 | 404 | ||
| 405 | 405 | // Git operations: bidirectional streaming via subprocess proxy | |
| 406 | - | if let Some((operation, raw_path)) = git::parse_git_command(&command_line) | |
| 407 | - | && let Some((owner, repo_name)) = git::parse_repo_path(raw_path) | |
| 408 | - | { | |
| 406 | + | if let Some(request) = git::parse_command(&command_line) { | |
| 407 | + | let operation = request.operation.command(); | |
| 408 | + | let (owner, repo_name) = (request.owner, request.repo); | |
| 409 | + | ||
| 409 | 410 | tracing::info!( | |
| 410 | 411 | user = %user.username, | |
| 411 | 412 | %operation, |
| @@ -1,0 +1,2 @@ | |||
| 1 | + | /target/ | |
| 2 | + | .DS_Store |
| @@ -1,0 +1,59 @@ | |||
| 1 | + | # This file is automatically @generated by Cargo. | |
| 2 | + | # It is not intended for manual editing. | |
| 3 | + | version = 4 | |
| 4 | + | ||
| 5 | + | [[package]] | |
| 6 | + | name = "git-command" | |
| 7 | + | version = "0.1.0" | |
| 8 | + | ||
| 9 | + | [[patch.unused]] | |
| 10 | + | name = "synckit-client" | |
| 11 | + | version = "0.8.0" | |
| 12 | + | ||
| 13 | + | [[patch.unused]] | |
| 14 | + | name = "synckit-config" | |
| 15 | + | version = "0.2.0" | |
| 16 | + | ||
| 17 | + | [[patch.unused]] | |
| 18 | + | name = "kberg" | |
| 19 | + | version = "0.1.0" | |
| 20 | + | ||
| 21 | + | [[patch.unused]] | |
| 22 | + | name = "ops-status" | |
| 23 | + | version = "0.1.0" | |
| 24 | + | ||
| 25 | + | [[patch.unused]] | |
| 26 | + | name = "painhours" | |
| 27 | + | version = "0.1.0" | |
| 28 | + | ||
| 29 | + | [[patch.unused]] | |
| 30 | + | name = "tagtree" | |
| 31 | + | version = "0.4.0" | |
| 32 | + | ||
| 33 | + | [[patch.unused]] | |
| 34 | + | name = "quasi-axum" | |
| 35 | + | version = "0.1.0" | |
| 36 | + | ||
| 37 | + | [[patch.unused]] | |
| 38 | + | name = "quasi-http" | |
| 39 | + | version = "0.1.0" | |
| 40 | + | ||
| 41 | + | [[patch.unused]] | |
| 42 | + | name = "quasi-router" | |
| 43 | + | version = "0.1.0" | |
| 44 | + | ||
| 45 | + | [[patch.unused]] | |
| 46 | + | name = "quasi-store" | |
| 47 | + | version = "0.1.0" | |
| 48 | + | ||
| 49 | + | [[patch.unused]] | |
| 50 | + | name = "quasi-tauri" | |
| 51 | + | version = "0.1.0" | |
| 52 | + | ||
| 53 | + | [[patch.unused]] | |
| 54 | + | name = "quasi-webview" | |
| 55 | + | version = "0.1.0" | |
| 56 | + | ||
| 57 | + | [[patch.unused]] | |
| 58 | + | name = "docengine" | |
| 59 | + | version = "0.7.0" |
| @@ -1,0 +1,45 @@ | |||
| 1 | + | [package] | |
| 2 | + | name = "git-command" | |
| 3 | + | version = "0.1.0" | |
| 4 | + | edition = "2024" | |
| 5 | + | # MIT rather than PolyForm. The threat model asks whether someone could collect | |
| 6 | + | # rent with just this crate and contribute nothing; a parser for the git-over-SSH | |
| 7 | + | # command grammar is not a service anyone can run. The perimeter is the MNW | |
| 8 | + | # server, and both consumers here stay PolyForm. | |
| 9 | + | license = "MIT" | |
| 10 | + | ||
| 11 | + | [dependencies] | |
| 12 | + | ||
| 13 | + | [dev-dependencies] | |
| 14 | + | ||
| 15 | + | [lints.rust] | |
| 16 | + | unused = "warn" | |
| 17 | + | unreachable_pub = "warn" | |
| 18 | + | ||
| 19 | + | [lints.clippy] | |
| 20 | + | pedantic = { level = "warn", priority = -1 } | |
| 21 | + | # Allow-list tuned from a measured breakdown across server/multithreaded/pter | |
| 22 | + | # (2026-07-22). These are the high-churn / low-signal pedantic lints; everything | |
| 23 | + | # else in `pedantic` stays a warning. Keep this block identical across repos. | |
| 24 | + | module_name_repetitions = "allow" | |
| 25 | + | # Doc lints. No docs-completeness push is underway. | |
| 26 | + | missing_errors_doc = "allow" | |
| 27 | + | missing_panics_doc = "allow" | |
| 28 | + | doc_markdown = "allow" | |
| 29 | + | # Numeric casts. Endemic and mostly intentional in size and byte math. | |
| 30 | + | cast_possible_truncation = "allow" | |
| 31 | + | cast_sign_loss = "allow" | |
| 32 | + | cast_precision_loss = "allow" | |
| 33 | + | cast_possible_wrap = "allow" | |
| 34 | + | cast_lossless = "allow" | |
| 35 | + | # Subjective structure and style nags. High churn, low signal. | |
| 36 | + | must_use_candidate = "allow" | |
| 37 | + | too_many_lines = "allow" | |
| 38 | + | struct_excessive_bools = "allow" | |
| 39 | + | similar_names = "allow" | |
| 40 | + | items_after_statements = "allow" | |
| 41 | + | single_match_else = "allow" | |
| 42 | + | # Frequent false-positives in TUI and router-heavy code. | |
| 43 | + | match_same_arms = "allow" | |
| 44 | + | unnecessary_wraps = "allow" | |
| 45 | + | type_complexity = "allow" |
| @@ -1,0 +1,50 @@ | |||
| 1 | + | # git-command | |
| 2 | + | ||
| 3 | + | The git-over-SSH command grammar, parsed once for every door that serves it. | |
| 4 | + | ||
| 5 | + | A git client asks for a repository by sending one line: | |
| 6 | + | ||
| 7 | + | git-upload-pack '/max/shop.git' | |
| 8 | + | ||
| 9 | + | Two hosts on this platform receive that line: `mnw-cli`'s russh server, which is | |
| 10 | + | what `ssh.makenot.work` runs, and `mnw-admin git-auth`, which sshd's `command=` | |
| 11 | + | prefix invokes. Until 2026-08-12 each parsed the line itself, and the two | |
| 12 | + | hand-written parsers disagreed on 51 of 5,424 measured commands. None of the | |
| 13 | + | divergences was an escape, because the charset guards downstream held on both | |
| 14 | + | sides. All of them meant the same push succeeded against one host and failed | |
| 15 | + | against the other. | |
| 16 | + | ||
| 17 | + | One grammar with two implementations is the defect. This crate is the grammar. | |
| 18 | + | ||
| 19 | + | ## What it promises | |
| 20 | + | ||
| 21 | + | **Path safety.** An accepted `Request` has an owner and a repo that are each a | |
| 22 | + | single, non-empty, non-traversing path segment, so `Request::repo_dir` cannot | |
| 23 | + | leave the root it is given and `Request::shell_command` cannot produce more than | |
| 24 | + | one quoted argument. | |
| 25 | + | ||
| 26 | + | It does not decide **identity policy** — whether `max` is a real user, whether a | |
| 27 | + | username may contain a hyphen, whether the caller may push here. That is the | |
| 28 | + | server's, and `Username::new` still runs on top of this. Path safety is a | |
| 29 | + | property of the string and belongs where the string is parsed; identity is a | |
| 30 | + | property of the deployment. | |
| 31 | + | ||
| 32 | + | ## Fuzzing | |
| 33 | + | ||
| 34 | + | The contract is executable, in `oracle::check`. Both the libFuzzer target and | |
| 35 | + | the committed regression replay call it, so neither can drift into checking less | |
| 36 | + | than the other. | |
| 37 | + | ||
| 38 | + | cargo test # unit tests + replay, stable | |
| 39 | + | cargo +nightly fuzz run command fuzz/corpus/command fuzz/seeds/command | |
| 40 | + | ||
| 41 | + | It is row 4 of the wiki note `astra-soak-overview`, the highest-severity target | |
| 42 | + | in that plan, and it runs on astra's soak tier as `git-command`. The oracle | |
| 43 | + | earned its keep on the first run: 90 seconds in, it found that a repo name | |
| 44 | + | ending in a dot produced a `git-shell` line this parser itself refused. | |
| 45 | + | ||
| 46 | + | ## License | |
| 47 | + | ||
| 48 | + | MIT. The threat model asks whether someone could collect rent with just this | |
| 49 | + | crate and contribute nothing; a parser for a command grammar is not a service | |
| 50 | + | anyone can run. Both consumers stay PolyForm Noncommercial. |
| @@ -1,0 +1,4 @@ | |||
| 1 | + | target | |
| 2 | + | corpus | |
| 3 | + | artifacts | |
| 4 | + | coverage |