michal/tit

Browse tree · Show commit · Download archive

Diff

24930372aded → 331b89a7fdfb

@@ -449,3 +449,17 @@
 and pull-request merge permission. Read the
 [branch-rule architectural decision record](docs/adr/0026-branch-rules.md) for
 the ref, role, transport, and merge contracts.
+
+## Milestone 5.6 gate
+
+Run the SSH pull-request command gate:
+
+```text
+./scripts/check-m5-6
+```
+
+This command tests bounded command parsing, exact human output, versioned JSON
+output, repository read permission, and the returned stock Git fetch and
+checkout commands. Read the
+[SSH pull-request checkout architectural decision record](docs/adr/0027-ssh-pull-request-checkout.md)
+for the command, output, permission, and error contracts.

@@ -1,0 +1,71 @@
+# Architectural decision record 0027: SSH pull-request checkout
+
+Status: Accepted
+
+Date: 2026-07-23
+
+## Context
+
+A contributor needs a stable command to fetch the current pull-request head.
+The command must use the same SSH authentication and repository read policy as
+the other SSH commands. Scripts also need output that does not depend on human
+text.
+
+## Decision
+
+Add this SSH command:
+
+```text
+pr checkout OWNER/REPOSITORY NUMBER [--output human|json]
+```
+
+The default output is exactly two Git commands:
+
+```text
+git fetch origin refs/pull/<number>/head:refs/heads/pr-<number>
+git checkout pr-<number>
+```
+
+The fetch command creates or updates the local `pr-<number>` branch from the
+server-owned pull-request ref. The checkout command selects that branch.
+
+The JSON output has version `1`. It contains the repository owner and name,
+the pull-request number, the remote ref, the local branch, and the two complete
+Git commands. A JSON error contains version `1`, status `error`, and one stable
+error code.
+
+Read the pull request through `Store::pull_request`. This method applies the
+current account, repository, visibility, and collaborator policy. The server
+recovers incomplete pull-request ref intents before it starts the SSH
+listener, so the command cannot observe an unrecovered ref intent.
+
+## Failure and threat cases
+
+Accept only ASCII command input with a bounded size. Require one repository
+target, one positive decimal number, and at most one output option. Reject
+extra input and duplicate options.
+
+Use the same `pull-request-unavailable` error for a missing pull request and
+one that the actor cannot read. This prevents private repository discovery.
+Return errors on standard error for human output and on standard output for
+JSON output.
+
+Do not run Git or OpenSSH in the server. The output is text for the client to
+run. The integration test uses stock Git only as an external test oracle.
+
+## Evidence
+
+The parser test checks valid human and JSON commands, invalid numbers, unsafe
+repository targets, duplicate options, extra input, and the size limit.
+
+The production SSH test creates a pull-request ref and metadata, checks the
+exact human output, and checks each JSON field. It then runs the returned fetch
+and checkout instructions with stock Git and verifies the selected object ID.
+The test also makes the repository private and verifies that an unauthorized
+account receives `pull-request-unavailable`.
+
+## Consequences
+
+The output contract is suitable for people and scripts. The local branch name
+is fixed to `pr-<number>`. A subsequent version must add a new JSON version or
+a new explicit option if it changes this naming contract.

@@ -1,0 +1,6 @@
+#!/bin/sh
+set -eu
+
+./scripts/check
+cargo test --locked --release --bin tit parses_only_bounded_pull_request_checkout_commands
+cargo test --locked --release --test serve creates_owned_repositories_with_stable_ssh_command_output

@@ -23,17 +23,21 @@
 use crate::issue::{IssueError, IssueService, MAX_BODY_BYTES, MAX_TITLE_BYTES};
 use crate::policy::RepositoryOperation;
 use crate::repository::{RepositoryService, RepositoryServiceError};
+use crate::store::{Store, StoreError};
 
 const VERSION_COMMAND: &[u8] = b"tit --version";
 const GIT_PROTOCOL_VARIABLE: &str = "GIT_PROTOCOL";
 const MAX_RECEIVE_PACK_BYTES: u64 = 128 * 1024 * 1024;
 const MAX_REPOSITORY_COMMAND_BYTES: usize = 512;
 const MAX_ISSUE_COMMAND_BYTES: usize = 512;
+const MAX_PULL_REQUEST_COMMAND_BYTES: usize = 512;
 const MAX_ISSUE_INPUT_BYTES: usize = MAX_TITLE_BYTES + 1 + MAX_BODY_BYTES;
 const REPOSITORY_CREATE_USAGE: &str =
     "repo create NAME [--object-format sha1|sha256] [--output human|json]";
 const ISSUE_LIST_USAGE: &str = "issue list OWNER/REPOSITORY [--output human|json]";
 const ISSUE_CREATE_USAGE: &str = "issue create OWNER/REPOSITORY [--output human|json]";
+const PULL_REQUEST_CHECKOUT_USAGE: &str =
+    "pr checkout OWNER/REPOSITORY NUMBER [--output human|json]";
 
 pub(crate) struct RunningSshServer {
     address: SocketAddr,
@@ -561,6 +565,19 @@
                     session,
                 )?,
             }
+        } else if is_pull_request_command(command) {
+            self.audit.accepted_exec.fetch_add(1, Ordering::Relaxed);
+            session.channel_success(channel)?;
+            let machine_requested = requests_json(command);
+            let result = match (parse_pull_request_command(command), self.active_identity()) {
+                (Ok(command), Some(identity)) => {
+                    run_pull_request_checkout(self.repositories.clone(), identity.username, command)
+                        .await
+                }
+                (Ok(_), None) => Err(PullRequestCommandError::Unavailable),
+                (Err(()), _) => Err(PullRequestCommandError::Usage),
+            };
+            send_pull_request_result(channel, result, machine_requested, session)?;
         } else {
             let service = self.open_git_service(command).await;
             if let Some(service) = service {
@@ -1356,6 +1373,174 @@
     }
 }
 
+struct PullRequestCheckoutCommand {
+    owner: String,
+    repository: String,
+    number: i64,
+    output: CommandOutput,
+}
+
+struct PullRequestCheckoutResult {
+    owner: String,
+    repository: String,
+    number: i64,
+    output: CommandOutput,
+}
+
+enum PullRequestCommandError {
+    Usage,
+    Unavailable,
+    Store(StoreError),
+}
+
+fn is_pull_request_command(command: &[u8]) -> bool {
+    command == b"pr" || command.starts_with(b"pr ")
+}
+
+fn parse_pull_request_command(command: &[u8]) -> Result<PullRequestCheckoutCommand, ()> {
+    if command.len() > MAX_PULL_REQUEST_COMMAND_BYTES || !command.is_ascii() {
+        return Err(());
+    }
+    let command = std::str::from_utf8(command).map_err(|_| ())?;
+    if command
+        .bytes()
+        .any(|byte| byte.is_ascii_control() && byte != b' ')
+    {
+        return Err(());
+    }
+    let mut tokens = command.split_ascii_whitespace();
+    if tokens.next() != Some("pr") || tokens.next() != Some("checkout") {
+        return Err(());
+    }
+    let target = tokens.next().ok_or(())?;
+    let (owner, repository) = target.split_once('/').ok_or(())?;
+    if owner.is_empty() || repository.is_empty() || repository.contains('/') {
+        return Err(());
+    }
+    let number = tokens.next().ok_or(())?.parse::<i64>().map_err(|_| ())?;
+    if number < 1 {
+        return Err(());
+    }
+    let output = parse_output_options(tokens)?;
+    Ok(PullRequestCheckoutCommand {
+        owner: owner.to_owned(),
+        repository: repository.to_owned(),
+        number,
+        output,
+    })
+}
+
+async fn run_pull_request_checkout(
+    repositories: Option<Arc<GitRepositories>>,
+    actor: String,
+    command: PullRequestCheckoutCommand,
+) -> Result<PullRequestCheckoutResult, PullRequestCommandError> {
+    let repositories = repositories.ok_or(PullRequestCommandError::Unavailable)?;
+    let database = repositories
+        .push_database()
+        .ok_or(PullRequestCommandError::Unavailable)?
+        .to_owned();
+    let permit = repositories
+        .blocking_permit()
+        .await
+        .map_err(|_| PullRequestCommandError::Unavailable)?;
+    tokio::task::spawn_blocking(move || {
+        let _permit = permit;
+        Store::open(&database)
+            .and_then(|store| {
+                store.pull_request(
+                    &command.owner,
+                    &command.repository,
+                    command.number,
+                    Some(&actor),
+                )
+            })
+            .map(|_| PullRequestCheckoutResult {
+                owner: command.owner,
+                repository: command.repository,
+                number: command.number,
+                output: command.output,
+            })
+            .map_err(PullRequestCommandError::Store)
+    })
+    .await
+    .map_err(|_| PullRequestCommandError::Unavailable)?
+}
+
+fn send_pull_request_result(
+    channel: ChannelId,
+    result: Result<PullRequestCheckoutResult, PullRequestCommandError>,
+    machine_requested: bool,
+    session: &mut Session,
+) -> Result<(), russh::Error> {
+    match result {
+        Ok(result) => {
+            let fetch = format!(
+                "git fetch origin refs/pull/{}/head:refs/heads/pr-{}",
+                result.number, result.number
+            );
+            let checkout = format!("git checkout pr-{}", result.number);
+            let data = match result.output {
+                CommandOutput::Human => format!("{fetch}\n{checkout}\n"),
+                CommandOutput::Json => format!(
+                    "{{\"version\":1,\"status\":\"success\",\"repository\":{{\"owner\":\"{}\",\"name\":\"{}\"}},\"pull_request\":{{\"number\":{},\"ref\":\"refs/pull/{}/head\",\"local_branch\":\"pr-{}\"}},\"commands\":{{\"fetch\":\"{}\",\"checkout\":\"{}\"}}}}\n",
+                    result.owner,
+                    result.repository,
+                    result.number,
+                    result.number,
+                    result.number,
+                    fetch,
+                    checkout,
+                ),
+            };
+            session.data(channel, data.into_bytes())?;
+            finish_git_channel(channel, 0, session)
+        }
+        Err(error) => {
+            if machine_requested {
+                session.data(
+                    channel,
+                    format!(
+                        "{{\"version\":1,\"status\":\"error\",\"error\":{{\"code\":\"{}\"}}}}\n",
+                        pull_request_command_error_code(&error)
+                    )
+                    .into_bytes(),
+                )?;
+            } else {
+                session.extended_data(
+                    channel,
+                    1,
+                    format!("tit: {}\n", pull_request_command_error_message(&error)).into_bytes(),
+                )?;
+            }
+            finish_git_channel(channel, 1, session)
+        }
+    }
+}
+
+fn pull_request_command_error_code(error: &PullRequestCommandError) -> &'static str {
+    match error {
+        PullRequestCommandError::Usage => "invalid-command",
+        PullRequestCommandError::Unavailable => "service-unavailable",
+        PullRequestCommandError::Store(
+            StoreError::PullRequestHidden
+            | StoreError::PullRequestNotFound(_, _, _)
+            | StoreError::RepositoryNotFound(_, _),
+        ) => "pull-request-unavailable",
+        PullRequestCommandError::Store(_) => "pull-request-checkout-failed",
+    }
+}
+
+fn pull_request_command_error_message(error: &PullRequestCommandError) -> String {
+    match pull_request_command_error_code(error) {
+        "invalid-command" => format!("usage: {PULL_REQUEST_CHECKOUT_USAGE}"),
+        "pull-request-unavailable" => "The pull request is not available.".to_owned(),
+        "invalid-target" => "The pull-request target is not valid.".to_owned(),
+        "service-unavailable" => "The pull-request service is not available.".to_owned(),
+        _ => "The pull-request checkout command could not be completed.".to_owned(),
+    }
+}
+
 impl SshSession {
     fn authorize(&self, public_key: &PublicKey) -> Auth {
         if self.authorized_keys.contains(public_key) {
@@ -1684,5 +1869,25 @@
             parse_issue_input(b"invalid\xff"),
             Err(IssueCommandError::Input)
         ));
+    }
+
+    #[test]
+    fn parses_only_bounded_pull_request_checkout_commands() {
+        let command = parse_pull_request_command(b"pr checkout alice/project 42 --output json")
+            .expect("parse a pull-request checkout command");
+        assert_eq!(command.owner, "alice");
+        assert_eq!(command.repository, "project");
+        assert_eq!(command.number, 42);
+        assert!(command.output == CommandOutput::Json);
+        for command in [
+            b"pr checkout alice/project 0".as_slice(),
+            b"pr checkout alice/project not-a-number".as_slice(),
+            b"pr checkout alice/project/extra 1".as_slice(),
+            b"pr checkout alice/project 1 --output json extra".as_slice(),
+            b"pr checkout alice/project 1 --output json --output json".as_slice(),
+            &[b'x'; MAX_PULL_REQUEST_COMMAND_BYTES + 1],
+        ] {
+            assert!(parse_pull_request_command(command).is_err());
+        }
     }
 }

@@ -604,6 +604,106 @@
         "example",
         &instance.path().join("created-clone")
     ));
+    let created_clone = instance.path().join("created-clone");
+    fs::write(created_clone.join("README.md"), b"base\n").expect("write pull-request base");
+    git_commit(&created_clone, "create main");
+    assert!(git_push(&member_key, &created_clone, &["main"]).success());
+    command(&created_clone, ["switch", "-q", "-c", "feature"]);
+    fs::write(created_clone.join("feature.txt"), b"feature\n").expect("write pull-request feature");
+    git_commit(&created_clone, "create feature");
+    assert!(git_push(&member_key, &created_clone, &["feature"]).success());
+    let base = git_revision(&created_clone, "main");
+    let head = git_revision(&created_clone, "feature");
+    let pull_request_database = rusqlite::Connection::open(instance.path().join("tit.sqlite3"))
+        .expect("open the pull-request command database");
+    let repository_id: String = pull_request_database
+        .query_row(
+            "SELECT id FROM repository WHERE slug = 'example'",
+            [],
+            |row| row.get(0),
+        )
+        .expect("read the pull-request repository ID");
+    let bare = instance
+        .path()
+        .join("repositories")
+        .join(format!("{repository_id}.git"));
+    command(
+        instance.path(),
+        [
+            "--git-dir",
+            bare.to_str().expect("a UTF-8 bare path"),
+            "update-ref",
+            "refs/pull/1/head",
+            &head,
+        ],
+    );
+    pull_request_database
+        .execute(
+            "INSERT INTO pull_request
+             (id, repository_id, number, title, body, state, author_account_id,
+              base_ref, head_ref, base_object_id, head_object_id, created_at, updated_at)
+             SELECT '11111111111111111111111111111111', ?1, 1, 'Feature', '', 'open',
+                    account.id, 'refs/heads/main', 'refs/heads/feature', ?2, ?3, 10, 10
+             FROM account WHERE username = 'bob'",
+            rusqlite::params![repository_id, base, head],
+        )
+        .expect("create a pull-request command fixture");
+    pull_request_database
+        .execute(
+            "INSERT INTO pull_request_revision
+             (id, pull_request_id, number, author_account_id, base_object_id,
+              head_object_id, created_at)
+             SELECT '22222222222222222222222222222222',
+                    '11111111111111111111111111111111', 1, account.id, ?1, ?2, 10
+             FROM account WHERE username = 'bob'",
+            rusqlite::params![base, head],
+        )
+        .expect("create a pull-request revision fixture");
+    drop(pull_request_database);
+
+    let human_checkout = ssh_exec(ssh, &member_key, &["pr", "checkout", "bob/example", "1"]);
+    assert!(human_checkout.status.success());
+    assert_eq!(
+        String::from_utf8(human_checkout.stdout).expect("read human checkout output"),
+        "git fetch origin refs/pull/1/head:refs/heads/pr-1\ngit checkout pr-1\n"
+    );
+    assert!(human_checkout.stderr.is_empty());
+    let machine_checkout = ssh_exec(
+        ssh,
+        &member_key,
+        &["pr", "checkout", "bob/example", "1", "--output", "json"],
+    );
+    assert!(machine_checkout.status.success());
+    let machine_checkout: serde_json::Value =
+        serde_json::from_slice(&machine_checkout.stdout).expect("parse machine checkout output");
+    assert_eq!(machine_checkout["version"], 1);
+    assert_eq!(machine_checkout["status"], "success");
+    assert_eq!(machine_checkout["repository"]["owner"], "bob");
+    assert_eq!(machine_checkout["repository"]["name"], "example");
+    assert_eq!(machine_checkout["pull_request"]["number"], 1);
+    assert_eq!(machine_checkout["pull_request"]["ref"], "refs/pull/1/head");
+    assert_eq!(
+        machine_checkout["commands"]["fetch"],
+        "git fetch origin refs/pull/1/head:refs/heads/pr-1"
+    );
+    let checkout_clone = instance.path().join("pull-request-checkout");
+    assert!(ssh_clone_repository_succeeds(
+        ssh,
+        &member_key,
+        "bob",
+        "example",
+        &checkout_clone
+    ));
+    assert!(
+        git_fetch_ref(
+            &member_key,
+            &checkout_clone,
+            "refs/pull/1/head:refs/heads/pr-1"
+        )
+        .success()
+    );
+    command(&checkout_clone, ["checkout", "-q", "pr-1"]);
+    assert_eq!(git_revision(&checkout_clone, "HEAD"), head);
 
     let machine = ssh_exec(
         ssh,
@@ -673,6 +773,17 @@
         serde_json::from_slice::<serde_json::Value>(&hidden.stdout)
             .expect("parse hidden issue error")["error"]["code"],
         "repository-unavailable"
+    );
+    let hidden_checkout = ssh_exec(
+        ssh,
+        &private_key,
+        &["pr", "checkout", "bob/example", "1", "--output", "json"],
+    );
+    assert!(!hidden_checkout.status.success());
+    assert_eq!(
+        serde_json::from_slice::<serde_json::Value>(&hidden_checkout.stdout)
+            .expect("parse hidden pull-request error")["error"]["code"],
+        "pull-request-unavailable"
     );
     access_database
         .execute(
@@ -1550,6 +1661,36 @@
         .current_dir(worktree)
         .output()
         .expect("push through the tit SSH server")
+}
+
+fn git_fetch_ref(private_key: &Path, worktree: &Path, refspec: &str) -> ExitStatus {
+    let ssh_command = format!(
+        "ssh -F /dev/null -i {} -o IdentitiesOnly=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null",
+        private_key.display()
+    );
+    Command::new("git")
+        .args(["fetch", "origin", refspec])
+        .env("GIT_SSH_COMMAND", ssh_command)
+        .current_dir(worktree)
+        .status()
+        .expect("fetch through the tit SSH server")
+}
+
+fn git_revision(worktree: &Path, revision: &str) -> String {
+    let output = Command::new("git")
+        .args(["rev-parse", revision])
+        .current_dir(worktree)
+        .output()
+        .expect("resolve a Git revision");
+    assert!(
+        output.status.success(),
+        "cannot resolve Git revision: {}",
+        String::from_utf8_lossy(&output.stderr)
+    );
+    String::from_utf8(output.stdout)
+        .expect("read a Git revision")
+        .trim()
+        .to_owned()
 }
 
 struct ChildGuard(Option<Child>);