From b388841a50872d5701c8f14052cb93213bd8b897 Mon Sep 17 00:00:00 2001 From: Karthik Th Date: Thu, 20 Aug 2026 15:40:06 +0530 Subject: [PATCH 001/926] Say why a pull request cannot be opened, and narrow the page The new pull request page redirected to the list when a repo had fewer than two branches. Clicking New pull request bounced straight back, which is what a broken link looks like -- it read as a missing page rather than as a repo with nothing to propose. It now says so. Container back to 1120px: 1400 read as too wide, with rows stretched far enough that tying a filename to its commit meant crossing the display. --- .../src/app/[owner]/[repo]/pulls/new/page.tsx | 29 +++++++++++++++++-- web/apps/web/src/app/globals.css | 6 ++-- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/web/apps/web/src/app/[owner]/[repo]/pulls/new/page.tsx b/web/apps/web/src/app/[owner]/[repo]/pulls/new/page.tsx index 9c525082..60d77fa2 100644 --- a/web/apps/web/src/app/[owner]/[repo]/pulls/new/page.tsx +++ b/web/apps/web/src/app/[owner]/[repo]/pulls/new/page.tsx @@ -1,4 +1,6 @@ -import { redirect } from "next/navigation"; +import Link from "next/link"; +import { GitBranch } from "lucide-react"; +import { Button } from "@/components/ui/button"; import { NewPullForm } from "@/components/repo/new-pull-form"; import { guardRepo } from "@/app/[owner]/[repo]/guard"; import { defaultBranch, refs, shortRef } from "@/lib/browse"; @@ -17,8 +19,29 @@ export default async function Page({ const all = await refs(token, owner, repo); if (!all.ok) throw new Error(all.message); const branches = all.value.filter((r) => r.kind === "branch").map((r) => shortRef(r.name)); - // Nothing to propose between: a repo with one branch has no second side. - if (branches.length < 2) redirect(`/${owner}/${repo}/pulls`); + // Nothing to propose between: a repo with one branch has no second side. Say + // so rather than redirecting — bouncing silently back to the list is what a + // broken link looks like, and the reader is left thinking the page is missing. + if (branches.length < 2) { + return ( +
+

New pull request

+
+ +

+ {branches.length === 1 ? `Only one branch` : `No branches yet`} +

+

+ A pull request proposes one branch onto another, so this repository + needs a second one. Push a branch and it will show up here. +

+ +
+
+ ); + } const fallback = defaultBranch(all.value); return ( diff --git a/web/apps/web/src/app/globals.css b/web/apps/web/src/app/globals.css index ea284f9e..3a1ce9ff 100644 --- a/web/apps/web/src/app/globals.css +++ b/web/apps/web/src/app/globals.css @@ -30,9 +30,9 @@ /* Measures. */ /* Wide enough for a file listing with a last-commit column beside an About - rail. 1120 left the code browser cramped on a laptop while the page sat in a - sea of margin. */ - --container-page: 1400px; + rail. 1400 was tried and read as too wide: past this, rows stretch and the + eye has to travel the whole width to tie a filename to its commit. */ + --container-page: 1120px; --container-prose: 560px; --container-headline: 640px; --container-readme: 760px; From 94ff55629ad028ddd4edb49580ed41dd4826a8f3 Mon Sep 17 00:00:00 2001 From: Karthik Th Date: Thu, 20 Aug 2026 16:17:07 +0530 Subject: [PATCH 002/926] Build the tree a patch produces A patch is one commit over many files, so the tree is rebuilt once for the whole set rather than once per path -- otherwise every directory on the way to a change is re-encoded once per file inside it. gitoxide's tree::Editor already does exactly this, so this is the git rules around it rather than a tree walk of our own. The rules are the part worth having: a path component of .. cannot be stored as a tree entry, but a client checking the tree out resolves it against the filesystem, which is a write outside the worktree. Same for .git. Writing over a symlink or a submodule is a corrupt tree rather than an edit, and an edit keeps the mode the file already has so a script does not silently stop being executable. The two tests are multi_thread because push_built serves the push from a task on the test runtime and then blocks the thread waiting for git; one thread cannot do both, and it deadlocks with no output. The git helper now also refuses to prompt, so a credential it cannot find fails loudly instead of hanging the suite the same way. --- deploy/rustic-git-web.yaml | 2 +- deploy/rustic-git.yaml | 6 +- src/objects.rs | 116 +++++++++++++++++++++++++++++++++++++ tests/common/mod.rs | 8 +++ tests/store.rs | 86 +++++++++++++++++++++++++++ 5 files changed, 214 insertions(+), 4 deletions(-) diff --git a/deploy/rustic-git-web.yaml b/deploy/rustic-git-web.yaml index e1e40c96..b5f21aaf 100644 --- a/deploy/rustic-git-web.yaml +++ b/deploy/rustic-git-web.yaml @@ -28,7 +28,7 @@ spec: # that can actually read ghcr.io/kloudlite/rustic-git-web. containers: - name: web - image: ghcr.io/kloudlite/rustic-git-web:2c990e4993d9b260b78ec92307a450d56215a064 + image: ghcr.io/kloudlite/rustic-git-web:b388841a50872d5701c8f14052cb93213bd8b897 ports: - { name: http, containerPort: 3000 } env: diff --git a/deploy/rustic-git.yaml b/deploy/rustic-git.yaml index 2207c367..03155f63 100644 --- a/deploy/rustic-git.yaml +++ b/deploy/rustic-git.yaml @@ -33,7 +33,7 @@ spec: topologyKey: kubernetes.io/hostname containers: - name: rustic-git - image: ghcr.io/kloudlite/rustic-git:ee77c6cab2d0840c17fec9b3ac5608fa66c49fd6 + image: ghcr.io/kloudlite/rustic-git:b388841a50872d5701c8f14052cb93213bd8b897 args: ["serve"] env: - name: RUSTIC_GIT_S3_URL @@ -227,7 +227,7 @@ spec: spec: containers: - name: api - image: ghcr.io/kloudlite/rustic-git:ee77c6cab2d0840c17fec9b3ac5608fa66c49fd6 + image: ghcr.io/kloudlite/rustic-git:b388841a50872d5701c8f14052cb93213bd8b897 # Its own binary, not a subcommand: this process cannot open a repository # for writing, because none of that code is linked into it. command: ["rustic-git-api"] @@ -298,7 +298,7 @@ spec: spec: containers: - name: worker - image: ghcr.io/kloudlite/rustic-git:ee77c6cab2d0840c17fec9b3ac5608fa66c49fd6 + image: ghcr.io/kloudlite/rustic-git:b388841a50872d5701c8f14052cb93213bd8b897 command: ["rustic-git-worker"] env: - name: RUSTIC_GIT_S3_URL diff --git a/src/objects.rs b/src/objects.rs index 06d7ed86..27875ab3 100644 --- a/src/objects.rs +++ b/src/objects.rs @@ -205,3 +205,119 @@ fn write_object_pack( std::fs::write(path, &out)?; Ok(()) } + +// ── patches ───────────────────────────────────────────────────────────────── + +/// What to do to one path in a patch. +pub enum Change { + /// Write these bytes there, creating the file or replacing what is there. + Upsert { + content: Vec, + /// `None` keeps the mode the file already has, so editing a script does + /// not quietly drop its executable bit. New files default to non-executable. + executable: Option, + }, + Delete, +} + +/// Build the tree that results from applying `changes` to `base`. +/// +/// A patch is ONE commit over many files, so the tree is rebuilt once for the +/// whole set rather than once per file — every directory on the way to a change +/// would otherwise be re-encoded once for each file inside it. +/// +/// The new blobs and trees are staged, not written: the caller writes them with +/// the commit, so a failure leaves no tree whose blobs are missing. +pub fn apply_changes( + odb: &(impl gix_object::FindExt + gix_object::Find), + base: Option, + changes: &std::collections::BTreeMap, + staging: &mut Staging, +) -> Result { + use gix_object::tree::EntryKind; + + if changes.is_empty() { + return Err(err("a commit needs at least one change")); + } + + let root = match base { + Some(oid) => { + let mut buf = Vec::new(); + gix_object::FindExt::find_tree(odb, &oid, &mut buf) + .map_err(|e| err(format!("reading the base tree: {e}")))? + .into() + } + None => gix_object::Tree::empty(), + }; + let mut editor = gix_object::tree::Editor::new(root, odb, gix_hash::Kind::Sha1); + + for (path, change) in changes { + let parts = split_path(path)?; + match change { + Change::Upsert { content, executable } => { + // The mode the path already has, so editing a script keeps its + // executable bit. A path that is a symlink or a submodule is + // refused: writing bytes over either is a corrupt tree, not an edit. + let kind = match editor.get(parts.iter()).map(|e| e.mode.kind()) { + Some(EntryKind::Link) => return Err(err(format!("{path} is a symbolic link"))), + Some(EntryKind::Commit) => return Err(err(format!("{path} is a submodule"))), + Some(EntryKind::Tree) => return Err(err(format!("{path} is a directory"))), + existing => match executable { + Some(true) => EntryKind::BlobExecutable, + Some(false) => EntryKind::Blob, + None if existing == Some(EntryKind::BlobExecutable) => { + EntryKind::BlobExecutable + } + None => EntryKind::Blob, + }, + }; + let blob = staging.add(gix_object::Kind::Blob, content.clone())?; + editor + .upsert(parts.iter(), kind, blob) + .map_err(|e| err(format!("{path}: {e}")))?; + } + // `remove_leaf`, not `remove`: deleting a path that turned out to be a + // directory would take everything under it with it, which is not what + // "delete this file" asked for. + Change::Delete => { + if editor.get(parts.iter()).is_none() { + return Err(err(format!("{path} is not in this branch"))); + } + editor + .remove_leaf(parts.iter()) + .map_err(|e| err(format!("{path}: {e}")))?; + } + } + } + + editor.write(|tree| { + let mut body = Vec::new(); + tree.write_to(&mut body)?; + staging.add(gix_object::Kind::Tree, body) + }) +} + +/// A path's components, refused unless every one of them is a name git will +/// store and a client will check out. +/// +/// `..` is the one that matters: a tree entry is a NAME, so a component that +/// means "the parent" cannot be stored — but a client checking the tree out +/// resolves it against the filesystem, which is a write outside the worktree. +fn split_path(path: &str) -> Result> { + if path.len() > 4096 { + return Err(err("path is too long")); + } + let parts: Vec<&str> = path.split('/').collect(); + for p in &parts { + let bad = p.is_empty() + || *p == "." + || *p == ".." + || p.eq_ignore_ascii_case(".git") + || p.contains('\\') + || p.bytes().any(|b| b < 0x20 || b == 0x7f); + if bad { + return Err(err(format!("{path} is not a valid path"))); + } + } + Ok(parts) +} diff --git a/tests/common/mod.rs b/tests/common/mod.rs index fb8ebd98..b43477fb 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -72,6 +72,14 @@ pub fn git(dir: &std::path::Path, args: &[&str]) -> String { .env("GIT_AUTHOR_EMAIL", "t@t") .env("GIT_COMMITTER_NAME", "t") .env("GIT_COMMITTER_EMAIL", "t@t") + // Never let git ask a human anything. A prompt here has no terminal to + // draw on and no one to answer it, so the subprocess blocks forever and + // the suite hangs with no output -- which reads as a deadlock in the code + // under test rather than as a credential git could not find. + .env("GIT_TERMINAL_PROMPT", "0") + .env("GIT_ASKPASS", "") + .env("SSH_ASKPASS", "") + .env("GCM_INTERACTIVE", "never") .output() .unwrap(); assert!( diff --git a/tests/store.rs b/tests/store.rs index a33777be..f769250c 100644 --- a/tests/store.rs +++ b/tests/store.rs @@ -417,3 +417,89 @@ async fn a_commit_the_server_writes_is_readable_afterwards() { .await; assert!(matches!(again, Ok(id) if id == oid), "idempotent: {again:?}"); } + +/// A patch is one commit over many files. These check the tree it builds is the +/// tree git would have built — including the parts that silently corrupt a repo +/// if they are wrong. +// multi_thread: push_built serves the push from a task on this runtime and +// then blocks the thread waiting for git, which one thread cannot do both of. +#[tokio::test(flavor = "multi_thread")] +async fn a_patch_edits_adds_and_deletes_in_one_commit() { + if !common::have_git() { eprintln!("skipping: no git"); return; } + use rustic_git::objects::{apply_changes, Change, Staging}; + use std::collections::BTreeMap; + + let e = common::env().await; + let repo = common::push_fixture(&e, "alice", "patched").await; + let s = &e.store; + let head = s.get_ref(&repo, "refs/heads/master").await.unwrap().unwrap(); + let odb = repo.odb().unwrap(); + let mut buf = Vec::new(); + let base = gix_object::FindExt::find_commit(&odb, &head, &mut buf).unwrap().tree(); + + // What the fixture starts with, so the assertions below are about the patch. + let before: Vec = rustic_git::browse::files_at(&odb, head, "", 1000) + .unwrap().into_iter().map(|e| e.name).collect(); + assert!(before.contains(&"src/main.rs".to_string()), "fixture has src/main.rs: {before:?}"); + + let mut changes = BTreeMap::new(); + // An edit deep in the tree, a new file in a directory that does not exist + // yet, and a delete -- all in ONE commit, which is the point of a patch. + changes.insert("src/main.rs".to_string(), Change::Upsert { content: b"edited\n".to_vec(), executable: None }); + changes.insert("deep/nested/new.txt".to_string(), Change::Upsert { content: b"new\n".to_vec(), executable: None }); + changes.insert("README.md".to_string(), Change::Delete); + + let mut staging = Staging::default(); + let tree = apply_changes(&odb, Some(base), &changes, &mut staging).unwrap(); + assert_ne!(tree, base, "the tree changed"); + + // Blobs and trees FIRST: a commit is validated against what is already + // stored, so writing it before the tree it points at asks the indexer to + // check an object against one that does not exist yet. + staging.write(s, &repo).await.unwrap(); + let oid = rustic_git::objects::write_commit(s, &repo, rustic_git::objects::NewCommit { + tree, parents: vec![head], message: "patch\n".into(), + author_name: "K".into(), author_email: "k@example.com".into(), time: 1_700_000_000, + }).await.unwrap(); + + let fresh = s.open_repo("alice", "patched").await.unwrap().unwrap(); + let odb2 = fresh.odb().unwrap(); + let after: Vec = rustic_git::browse::files_at(&odb2, oid, "", 1000) + .unwrap().into_iter().map(|e| e.name).collect(); + assert!(after.contains(&"deep/nested/new.txt".to_string()), "new nested file: {after:?}"); + assert!(after.contains(&"src/main.rs".to_string()), "edited file still there: {after:?}"); + assert!(!after.contains(&"README.md".to_string()), "deleted file is gone: {after:?}"); + + // The edit is really the new bytes, read back through a fresh handle. + let blob = rustic_git::browse::blob_at(&odb2, oid, "src/main.rs", 1 << 20).unwrap(); + assert_eq!(blob.bytes, b"edited\n", "the edit landed"); +} + +// multi_thread: push_built serves the push from a task on this runtime and +// then blocks the thread waiting for git, which one thread cannot do both of. +#[tokio::test(flavor = "multi_thread")] +async fn a_patch_refuses_a_path_that_escapes_the_tree() { + if !common::have_git() { eprintln!("skipping: no git"); return; } + use rustic_git::objects::{apply_changes, Change, Staging}; + use std::collections::BTreeMap; + + let e = common::env().await; + let repo = common::push_fixture(&e, "alice", "escapes").await; + let head = e.store.get_ref(&repo, "refs/heads/master").await.unwrap().unwrap(); + let odb = repo.odb().unwrap(); + let mut buf = Vec::new(); + let base = gix_object::FindExt::find_commit(&odb, &head, &mut buf).unwrap().tree(); + + // A tree entry is a NAME, so these cannot be stored -- but a client checking + // the tree out resolves them against the filesystem, which is a write + // outside the worktree. They are refused rather than normalised. + for path in ["../escape.txt", "a/../../escape.txt", ".git/config", "a//b.txt", "", "./x"] { + let mut changes = BTreeMap::new(); + changes.insert(path.to_string(), Change::Upsert { content: b"x".to_vec(), executable: None }); + let mut staging = Staging::default(); + assert!( + apply_changes(&odb, Some(base), &changes, &mut staging).is_err(), + "{path:?} must be refused", + ); + } +} From 074a22abb4cba63aaa4e5131091d13e9a3195830 Mon Sep 17 00:00:00 2001 From: Karthik Th Date: Thu, 20 Aug 2026 16:22:05 +0530 Subject: [PATCH 003/926] Accept a patch across many files as one commit POST /v1/repos/{owner}/{name}/commits takes a set of changes -- upserts and deletes, contents base64 because a file is arbitrary bytes and JSON carries text -- and lands them as a single commit. Three things it refuses rather than guesses. The author is whoever is signed in, overwritten from the verified token before forwarding: a caller that could name its own author could write history as somebody else. The branch tip is re-read on the owning node and compared against what the editor was reading, so a push that arrives mid-edit loses the race instead of being overwritten. And a patch whose tree comes out equal to the base is 'this changes nothing', not an empty commit. newBranch lands the commit on a new branch instead, leaving the base untouched -- which is how an edit to a protected branch becomes a change to review rather than a refusal. The api tier still writes nothing: it authorizes, names the author and forwards. update_refs on the owning node keeps the last word. --- src/api.rs | 76 +++++++++++++++++ src/http/browse_api.rs | 186 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 261 insertions(+), 1 deletion(-) diff --git a/src/api.rs b/src/api.rs index b02d47fc..bc48e112 100644 --- a/src/api.rs +++ b/src/api.rs @@ -116,6 +116,7 @@ pub async fn serve( axum::routing::post(comment_on_pull), ) .route("/v1/repos/{owner}/{name}/pulls/{number}/merge", axum::routing::post(merge_pull)) + .route("/v1/repos/{owner}/{name}/commits", axum::routing::post(commit_patch)) .route("/v1/repos/{owner}/{name}/pulls/{number}/close", axum::routing::post(close_pull)) .route("/v1/repos/{owner}/{name}/compare", axum::routing::get(compare_branches)) .route( @@ -1962,6 +1963,81 @@ struct SignatureOf { author_email: String, } +/// The api tier's half of a patch: authorize, name the author, forward. +/// +/// The api tier never writes objects itself — the owning node does, because one +/// writer per repo is what makes branch protection and ref updates decidable. So +/// this establishes WHO is committing and hands the patch on; the node's +/// `update_refs` still has the last word on whether the branch may move. +async fn commit_patch( + State(api): State>, + axum::extract::Path((owner, name)): axum::extract::Path<(String, String)>, + headers: axum::http::HeaderMap, + axum::Json(mut body): axum::Json, +) -> Response { + let user = match caller(&api, &headers) { + Ok(u) => u, + Err(r) => return r, + }; + if let Err(r) = settings_caller(&api, &headers, &owner, &name).await { + return r; + } + + // The author is WHO IS SIGNED IN, never what the request said. A caller that + // could name its own author could write history as somebody else. + let name_of = api + .jwt + .as_deref() + .and_then(|j| { + headers + .get(axum::http::header::AUTHORIZATION) + .and_then(|v| v.to_str().ok()) + .and_then(|v| v.strip_prefix("Bearer ")) + .and_then(|t| j.verify(t.trim()).ok()) + }) + .map(|c| c.name) + .unwrap_or_else(|| user.clone()); + let Some(obj) = body.as_object_mut() else { + return (StatusCode::BAD_REQUEST, "expected an object").into_response(); + }; + obj.insert("authorName".into(), serde_json::Value::String(name_of)); + obj.insert("authorEmail".into(), serde_json::Value::String(user)); + + let url = format!("{}/api/{}/{}/patch", api.upstream, encode(&owner), encode(&name)); + let sent = api + .client + .post(url) + .header(crate::proxy::PEER_HEADER, &api.secret) + .header(crate::proxy::OWNER_HEADER, &owner) + .header(axum::http::header::CONTENT_TYPE, "application/json") + .body(match serde_json::to_vec(&body) { + Ok(b) => b, + Err(e) => { + eprintln!("commit patch: {e}"); // ponytail: eprintln + return (StatusCode::BAD_REQUEST, "could not read the patch").into_response(); + } + }) + .send() + .await; + let r = match sent { + Ok(r) => r, + Err(e) => { + eprintln!("commit patch: {e}"); // ponytail: eprintln + return (StatusCode::BAD_GATEWAY, "could not reach the repository").into_response(); + } + }; + let status = StatusCode::from_u16(r.status().as_u16()).unwrap_or(StatusCode::BAD_GATEWAY); + let text = r.text().await.unwrap_or_default(); + // The node's own words: "this branch has moved since you started editing", or + // the protection rule that refused it. Both are written for the person at the + // editor, so they are passed through rather than replaced. + if status.is_success() { + (status, [(axum::http::header::CONTENT_TYPE, "application/json")], text).into_response() + } else { + (status, text).into_response() + } +} + async fn verify_commit( State(api): State>, axum::extract::Path((owner, name, sha)): axum::extract::Path<(String, String, String)>, diff --git a/src/http/browse_api.rs b/src/http/browse_api.rs index d644c2b8..e3421c6c 100644 --- a/src/http/browse_api.rs +++ b/src/http/browse_api.rs @@ -14,7 +14,7 @@ use axum::{ Json, Router, }; use gix_hash::ObjectId; -use serde::Serialize; +use serde::{Deserialize, Serialize}; use std::collections::HashMap; use std::sync::Arc; @@ -609,6 +609,184 @@ async fn api_merge( } } +/// One file's worth of a patch, as the api tier sends it. +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct FileChange { + path: String, + /// Base64: a file is arbitrary bytes and JSON carries text, so the bytes + /// cannot go over as a string. Absent when `delete` is set. + content_base64: Option, + /// `None` keeps the mode the file already has. + executable: Option, + #[serde(default)] + delete: bool, +} + +/// A patch: one commit, any number of files. +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct Patch { + /// The branch the editor was reading. + branch: String, + /// The tip it was reading, if the caller knows it. The commit is refused if + /// the branch has moved since — someone pushed while the editor was open, + /// and landing on top of a tip we never saw would silently drop their work. + expect: Option, + message: String, + author_name: String, + author_email: String, + /// Commit onto a NEW branch of this name instead of moving `branch`. This is + /// what "start a pull request from this edit" is: the base branch does not + /// move at all, so it can be protected and the edit still lands. + new_branch: Option, + changes: Vec, +} + +#[derive(Serialize)] +struct Committed { + commit: String, + branch: String, +} + +/// Apply a patch as one commit. +/// +/// The whole patch lands or none of it does: the blobs and trees are staged and +/// written together, and the ref moves only once the commit is stored. And the +/// ref moves by compare-and-swap, so a push that arrives mid-edit loses the race +/// rather than being overwritten. +async fn api_patch( + State(app): State>, + Path((owner, name)): Path<(String, String)>, + Json(patch): Json, +) -> Response { + let Some((owner, name)) = crate::protocol::parse_repo_path(&format!("{owner}/{name}")) else { + return (StatusCode::BAD_REQUEST, "invalid repository path").into_response(); + }; + let repo = match app.store.open_repo(&owner, &name).await { + Ok(Some(r)) => r, + Ok(None) => return hidden(), + Err(e) => return internal(e), + }; + if patch.changes.is_empty() { + return (StatusCode::BAD_REQUEST, "a commit needs at least one change").into_response(); + } + if patch.message.trim().is_empty() { + return (StatusCode::BAD_REQUEST, "a commit needs a message").into_response(); + } + + let branch_ref = format!("refs/heads/{}", patch.branch); + let tip = match app.store.get_ref(&repo, &branch_ref).await { + Ok(t) => t, + Err(e) => return internal(e), + }; + // Read HERE rather than trusted from whatever the editor last saw. + if let Some(expected) = &patch.expect { + if tip.map(|t| t.to_hex().to_string()).as_deref() != Some(expected.as_str()) { + return ( + StatusCode::CONFLICT, + "this branch has moved since you started editing", + ) + .into_response(); + } + } + let Some(tip) = tip else { + return (StatusCode::NOT_FOUND, "no such branch").into_response(); + }; + + let odb = match repo.odb() { + Ok(o) => o, + Err(e) => return internal(e), + }; + let mut buf = Vec::new(); + let base_tree = match gix_object::FindExt::find_commit(&odb, &tip, &mut buf) { + Ok(c) => c.tree(), + Err(e) => return internal(crate::err(e.to_string())), + }; + + let mut changes = std::collections::BTreeMap::new(); + for c in patch.changes { + let change = if c.delete { + crate::objects::Change::Delete + } else { + use base64::Engine; + let Some(b64) = c.content_base64.as_deref() else { + return (StatusCode::BAD_REQUEST, format!("{}: no content", c.path)).into_response(); + }; + match base64::engine::general_purpose::STANDARD.decode(b64) { + Ok(content) => crate::objects::Change::Upsert { content, executable: c.executable }, + Err(_) => { + return (StatusCode::BAD_REQUEST, format!("{}: content is not base64", c.path)) + .into_response() + } + } + }; + // Two changes to one path have no defined order, so the patch is refused + // rather than one of them silently winning. + if changes.insert(c.path.clone(), change).is_some() { + return (StatusCode::BAD_REQUEST, format!("{} appears twice", c.path)).into_response(); + } + } + + let mut staging = crate::objects::Staging::default(); + let tree = match crate::objects::apply_changes(&odb, Some(base_tree), &changes, &mut staging) { + Ok(t) => t, + Err(e) => return (StatusCode::BAD_REQUEST, e.to_string()).into_response(), + }; + // Nothing actually changed: the same bytes were sent back. A commit here + // would be an empty one, which is noise in the history rather than a record. + if tree == base_tree { + return (StatusCode::BAD_REQUEST, "this changes nothing").into_response(); + } + + // Blobs and trees FIRST: a commit is validated against what is stored, so it + // cannot be written before the tree it points at. + if let Err(e) = staging.write(&app.store, &repo).await { + return internal(e); + } + let time = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs() as i64) + .unwrap_or(0); + let commit = match crate::objects::write_commit( + &app.store, + &repo, + crate::objects::NewCommit { + tree, + parents: vec![tip], + message: patch.message, + author_name: patch.author_name, + author_email: patch.author_email, + time, + }, + ) + .await + { + Ok(c) => c, + Err(e) => return internal(e), + }; + + // Onto a new branch, the update is a CREATE (`old: None`), so it cannot + // overwrite a branch of that name that already exists. + let (target, old) = match &patch.new_branch { + Some(b) => (format!("refs/heads/{b}"), None), + None => (branch_ref, Some(tip)), + }; + let landed_on = patch.new_branch.clone().unwrap_or(patch.branch); + match app + .store + .update_refs(&repo, &[crate::refs::RefUpdate { name: target, old, new: Some(commit) }]) + .await + { + Ok(r) => match r.into_iter().next().flatten() { + None => Json(Committed { commit: commit.to_hex().to_string(), branch: landed_on }) + .into_response(), + Some(reason) => (StatusCode::CONFLICT, reason).into_response(), + }, + Err(e) => internal(e), + } +} + #[derive(Serialize)] struct SignatureOf { signature: String, @@ -678,6 +856,12 @@ pub fn browse_routes() -> Router> { "/api/{owner}/{name}/create", post(api_create).layer(axum::extract::DefaultBodyLimit::max(0)), ) + // A patch carries file contents, so this is the one write route with a real + // body: 25 MiB, which is a generous edit and far below what a push is for. + .route( + "/api/{owner}/{name}/patch", + post(api_patch).layer(axum::extract::DefaultBodyLimit::max(25 * 1024 * 1024)), + ) .route( "/api/{owner}/{name}/delete", post(api_delete).layer(axum::extract::DefaultBodyLimit::max(0)), From a53bb828da6a5408ef63f39827812218eb79f691 Mon Sep 17 00:00:00 2001 From: Karthik Th Date: Thu, 20 Aug 2026 16:24:44 +0530 Subject: [PATCH 004/926] Edit a file in the browser and commit it A pencil on the blob view opens the file in a textarea, with the two places a commit can go: straight onto the branch, or onto a new one which then opens the pull request. The second is what makes editing a protected branch possible at all -- the base never moves, so the change goes to review instead of being refused. Deliberately a textarea, not a code editor. This is for a typo, a version bump, a line in a README; anything larger is a checkout, and pretending otherwise means shipping a syntax engine to edit one line. The Edit button appears only where an edit can actually be made: not on binary, which a textarea would turn into mojibake and commit; not on a truncated blob, which would commit the part that was served and drop the rest; and not on a tag, which has no branch for the commit to land on. The tip being read is sent with the commit, so a push that lands mid-edit is a conflict the person is told about. --- .../[owner]/[repo]/edit/[...path]/page.tsx | 50 +++++++ .../src/app/[owner]/[repo]/edit/actions.ts | 58 ++++++++ .../web/src/components/repo/file-editor.tsx | 135 ++++++++++++++++++ .../web/src/components/repo/file-view.tsx | 13 +- web/apps/web/src/lib/api.ts | 34 +++++ 5 files changed, 289 insertions(+), 1 deletion(-) create mode 100644 web/apps/web/src/app/[owner]/[repo]/edit/[...path]/page.tsx create mode 100644 web/apps/web/src/app/[owner]/[repo]/edit/actions.ts create mode 100644 web/apps/web/src/components/repo/file-editor.tsx diff --git a/web/apps/web/src/app/[owner]/[repo]/edit/[...path]/page.tsx b/web/apps/web/src/app/[owner]/[repo]/edit/[...path]/page.tsx new file mode 100644 index 00000000..d480bc80 --- /dev/null +++ b/web/apps/web/src/app/[owner]/[repo]/edit/[...path]/page.tsx @@ -0,0 +1,50 @@ +import { notFound, redirect } from "next/navigation"; +import { FileEditor } from "@/components/repo/file-editor"; +import { guardRepo } from "@/app/[owner]/[repo]/guard"; +import { blob, decodeBlob, defaultBranch, refs, shortRef } from "@/lib/browse"; + +export default async function Page({ + params, + searchParams, +}: { + params: Promise<{ owner: string; repo: string; path: string[] }>; + searchParams: Promise<{ ref?: string }>; +}) { + const { owner, repo, path } = await params; + const { token } = await guardRepo(owner, repo); + const { ref } = await searchParams; + const file = path.join("/"); + + const all = await refs(token, owner, repo); + if (!all.ok) throw new Error(all.message); + const fallback = defaultBranch(all.value); + const head = (ref && all.value.find((r) => shortRef(r.name) === ref)) || fallback; + if (!head) throw new Error("this repo has no branches"); + const branch = shortRef(head.name); + + // Editing is editing a BRANCH. A tag or a bare commit has nothing to move, so + // there is nowhere for the edit to land. + if (head.kind !== "branch") redirect(`/${owner}/${repo}/blob/${file}?ref=${encodeURIComponent(branch)}`); + + const b = await blob(token, owner, repo, head.oid, file); + if (!b.ok) notFound(); + const decoded = decodeBlob(b.value); + // Binary is not text, and a textarea would turn it into mojibake and commit + // that. Say so where they clicked rather than opening an editor that corrupts. + if (decoded.binary || b.value.truncated) { + redirect(`/${owner}/${repo}/blob/${file}?ref=${encodeURIComponent(branch)}`); + } + + return ( + + ); +} diff --git a/web/apps/web/src/app/[owner]/[repo]/edit/actions.ts b/web/apps/web/src/app/[owner]/[repo]/edit/actions.ts new file mode 100644 index 00000000..749cabac --- /dev/null +++ b/web/apps/web/src/app/[owner]/[repo]/edit/actions.ts @@ -0,0 +1,58 @@ +"use server"; + +import { revalidatePath } from "next/cache"; +import { redirect } from "next/navigation"; +import { apiToken } from "@/lib/api-token"; +import * as api from "@/lib/api"; + +export type EditState = { error?: string } | null; + +/** Commit an edited file. + * + * The choice of where it lands is the form's: onto the branch being viewed, or + * onto a new one so it can be reviewed first. Everything else -- who the author + * is, whether the branch moved, whether protection allows it -- is decided by + * the server, which is why none of it is sent from here. */ +export async function commitFile(_prev: EditState, formData: FormData): Promise { + const owner = String(formData.get("owner") ?? ""); + const repo = String(formData.get("repo") ?? ""); + const branch = String(formData.get("branch") ?? ""); + const path = String(formData.get("path") ?? ""); + const expect = String(formData.get("expect") ?? "") || undefined; + const content = String(formData.get("content") ?? ""); + const target = String(formData.get("target") ?? "here"); + const newBranch = String(formData.get("newBranch") ?? "").trim(); + + const message = + String(formData.get("message") ?? "").trim() || `Update ${path.split("/").pop() ?? path}`; + + if (target === "branch" && !newBranch) return { error: "Name the new branch." }; + if (target === "branch" && newBranch === branch) { + return { error: "That is the branch you are already on." }; + } + + const token = await apiToken(); + if (!token) return { error: "Your session has expired. Sign in again." }; + + // A textarea gives back a JS string; the server wants the file's bytes. UTF-8 + // first, so anything outside Latin-1 survives the trip. + const contentBase64 = Buffer.from(content, "utf8").toString("base64"); + + const r = await api.commitPatch(token, owner, repo, { + branch, + message, + expect, + newBranch: target === "branch" ? newBranch : undefined, + changes: [{ path, contentBase64 }], + }); + if (!r.ok) return { error: r.message || "Could not commit." }; + + const landed = r.value.branch; + revalidatePath(`/${owner}/${repo}`, "layout"); + // Onto a new branch, the next thing anyone wants is the pull request -- that is + // why they chose a branch rather than committing here. + if (target === "branch") { + redirect(`/${owner}/${repo}/pulls/new?base=${encodeURIComponent(branch)}&head=${encodeURIComponent(landed)}`); + } + redirect(`/${owner}/${repo}/blob/${path}?ref=${encodeURIComponent(landed)}`); +} diff --git a/web/apps/web/src/components/repo/file-editor.tsx b/web/apps/web/src/components/repo/file-editor.tsx new file mode 100644 index 00000000..5e37c960 --- /dev/null +++ b/web/apps/web/src/components/repo/file-editor.tsx @@ -0,0 +1,135 @@ +"use client"; + +import Link from "next/link"; +import { useActionState, useState } from "react"; +import { GitBranch, GitCommitHorizontal, Loader2 } from "lucide-react"; +import { Button } from "@/components/ui/button"; +import { Input } from "@/components/ui/input"; +import { Textarea } from "@/components/ui/textarea"; +import { FieldLabel } from "@/components/auth/auth-card"; +import { commitFile, type EditState } from "@/app/[owner]/[repo]/edit/actions"; + +/** A file, and somewhere to put it. + * + * Deliberately a textarea rather than a code editor: this is for a typo, a + * version bump, a line in a README. Anything larger is a checkout, and pretending + * otherwise would mean shipping a syntax engine to edit one line. + * + * The two places an edit can go are the whole point of the second half of this + * form. Committing straight to the branch is the fast path; a new branch is what + * makes an edit to a protected branch possible at all, since the base never + * moves and the change goes to review instead. */ +export function FileEditor({ + owner, + repo, + path, + branch, + expect, + initial, +}: { + owner: string; + repo: string; + path: string; + branch: string; + expect: string; + initial: string; +}) { + const [state, action, pending] = useActionState(commitFile, null); + const [target, setTarget] = useState<"here" | "branch">("here"); + const [text, setText] = useState(initial); + const filename = path.split("/").pop() ?? path; + const untouched = text === initial; + const suggestion = `patch-${filename.replace(/[^a-zA-Z0-9._-]/g, "-")}`; + + return ( +
+
+

{path}

+ on {branch} +
+ +
+ + + + + + + +