fix: keep the GitHub token out of build containers, lock down node edges

docs/GITHUB.md

@@ -23,9 +23,10 @@ openpages login          # wraps `gh auth login` with the right scopes
 ```
 
 Everything after that reads the token with `gh auth token`, per operation. The
-token is never written to the openpages database. It is passed to a build only
-as part of a single `git clone` URL, and every log line is redacted against it
-before being stored or shown.
+token is never written to the openpages database or to disk. git receives it
+as an `http.extraheader` for github.com in its environment, so it never lands in
+`.git/config` inside the workspace that the build container mounts, and every
+log line is redacted against it before being stored or shown.
 
 To switch accounts:
 
@@ -91,6 +92,6 @@ anything that could remove code.
 | List repositories | `gh repo list --source --json …` |
 | Default branch, branch head | `gh api repos/{repo}` |
 | Add / remove a push webhook | `gh api repos/{repo}/hooks` |
-| Clone for a build | `git clone` with a token in the URL |
+| Clone for a build | `git fetch` with the token as an HTTP header (environment, not URL) |
 | Open a repair pull request | `gh pr create` |
 | Check for an openpages release | `gh api repos/{repo}/releases/latest` |

docs/NODES.md

@@ -43,8 +43,12 @@ x-openpages-target: static:blog/412
 x-openpages-target: container:38211
 ```
 
-The node has no database, so it is told rather than asked to look up. A request
-to a node's edge without that header is refused — it is not a general proxy.
+The node has no database, so it is told rather than asked to look up. Each
+request also carries `x-openpages-node-key`, the SHA-256 of the node's secret
+that the panel stored at enrolment; the node recomputes it from its own token.
+A request without both headers is refused — it is not a general proxy. The
+panel never forwards a visitor's own copies of these headers. Panel and node
+must be upgraded together: a node with this check refuses an older panel.
 
 Give `--edge-url` an address the panel can reach: a LAN address, a tailnet
 address, a private VPC address. If the node is genuinely unreachable, keep its

src/api.rs

@@ -872,7 +872,12 @@ async fn webhook(
 
     let push: PushEvent =
         serde_json::from_str(&body).map_err(|e| bad(format!("unreadable push event: {e}")))?;
-    let branch = push.git_ref.rsplit('/').next().unwrap_or("").to_string();
+    // Only branch pushes deploy. `rsplit('/')` used to turn `refs/heads/fix/main`
+    // into `main` (a production deploy of a feature branch) and tags into
+    // branches.
+    let Some(branch) = push.git_ref.strip_prefix("refs/heads/").map(str::to_string) else {
+        return Ok(Json(json!({ "ok": true, "ignored": push.git_ref })));
+    };
     let mut started = Vec::new();
 
     for site in db::site_by_repo(&state.db, &push.repository.full_name).await? {
@@ -1004,7 +1009,11 @@ async fn node_logs(
     headers: HeaderMap,
     Json(b): Json<NodeLogs>,
 ) -> ApiResult<Json<serde_json::Value>> {
-    node_auth(&state, &headers).await?;
+    let node = node_auth(&state, &headers).await?;
+    let d = db::deployment(&state.db, b.deployment_id)
+        .await?
+        .ok_or_else(|| missing("deployment"))?;
+    claimed_by(&d, &node)?;
     db::append_logs(&state.db, b.deployment_id, &b.lines).await?;
     Ok(Json(json!({ "ok": true })))
 }
@@ -1014,10 +1023,17 @@ async fn node_result(
     headers: HeaderMap,
     Json(out): Json<crate::models::BuildOutcome>,
 ) -> ApiResult<Json<serde_json::Value>> {
-    node_auth(&state, &headers).await?;
+    let node = node_auth(&state, &headers).await?;
     let d = db::deployment(&state.db, out.deployment_id)
         .await?
         .ok_or_else(|| missing("deployment"))?;
+    claimed_by(&d, &node)?;
+    if d.state != "building" {
+        return Err(Fail(
+            StatusCode::CONFLICT,
+            "that deployment is not building".into(),
+        ));
+    }
     let site = db::site(&state.db, d.site_id)
         .await?
         .ok_or_else(|| missing("site"))?;
@@ -1051,6 +1067,16 @@ async fn node_result(
     Ok(Json(json!({ "ok": true })))
 }
 
+/// A node may only report on a deployment that was queued for it. Without
+/// this, any node token could mark a local deployment ready with an
+/// `artifact_path` of `/`, or a `container_port` of any port on the panel.
+fn claimed_by(d: &crate::models::Deployment, node: &crate::models::Node) -> ApiResult<()> {
+    if d.node_id.as_deref().unwrap_or("local") != node.id {
+        return Err(missing("deployment"));
+    }
+    Ok(())
+}
+
 // ── the panel itself ────────────────────────────────────────────────────────
 
 #[derive(rust_embed::RustEmbed)]

src/buildpipe.rs

@@ -34,7 +34,13 @@ pub struct BuildEnv {
 pub type LogSink = mpsc::Sender<(String, String)>;
 
 pub async fn run(job: &BuildJob, env: &BuildEnv, sink: LogSink) -> BuildOutcome {
-    let secrets: Vec<String> = job.clone_token.iter().cloned().collect();
+    let mut secrets: Vec<String> = job.clone_token.iter().cloned().collect();
+    // The header form of the token, in case git ever echoes it.
+    if let Some(t) = &job.clone_token {
+        use base64::Engine;
+        secrets
+            .push(base64::engine::general_purpose::STANDARD.encode(format!("x-access-token:{t}")));
+    }
     let recorder = Recorder::new(sink, secrets.clone());
 
     match run_inner(job, env, &recorder).await {
@@ -106,10 +112,18 @@ async fn run_inner(job: &BuildJob, env: &BuildEnv, rec: &Recorder) -> Result<Bui
 /// Fetch exactly one commit. Not the branch tip: between the webhook firing and
 /// the build starting, the branch can move, and a deployment labelled `abc1234`
 /// that actually contains `def5678` is a debugging session nobody deserves.
+///
+/// The token never touches the disk. It used to be part of the remote URL,
+/// which git writes into `.git/config` — inside the workspace that is then
+/// bind-mounted into the build container, where every install script could
+/// read it. It now travels as an `http.extraheader` in the environment of the
+/// git processes that need it, scoped to github.com.
 async fn clone(job: &BuildJob, work: &Path, rec: &Recorder) -> Result<()> {
-    let url = gh::clone_url(&job.repo, job.clone_token.as_deref());
-    git(work, &["init", "--quiet"], rec).await?;
-    git(work, &["remote", "add", "origin", &url], rec).await?;
+    let url = gh::clone_url(&job.repo, None);
+    let auth = job.clone_token.as_deref().map(gh::auth_env);
+    let auth = auth.as_deref();
+    git(work, &["init", "--quiet"], None, rec).await?;
+    git(work, &["remote", "add", "origin", &url], None, rec).await?;
     git(
         work,
         &[
@@ -120,6 +134,7 @@ async fn clone(job: &BuildJob, work: &Path, rec: &Recorder) -> Result<()> {
             "origin",
             &job.commit_sha,
         ],
+        auth,
         rec,
     )
     .await
@@ -133,6 +148,7 @@ async fn clone(job: &BuildJob, work: &Path, rec: &Recorder) -> Result<()> {
     git(
         work,
         &["checkout", "--quiet", "--detach", "FETCH_HEAD"],
+        None,
         rec,
     )
     .await?;
@@ -150,20 +166,30 @@ async fn clone(job: &BuildJob, work: &Path, rec: &Recorder) -> Result<()> {
                 "1",
                 "--recursive",
             ],
+            auth,
             rec,
         )
         .await?;
     }
     Ok(())
 }
 
-async fn git(dir: &Path, args: &[&str], rec: &Recorder) -> Result<()> {
-    let out = Command::new("git")
-        .args(args)
+async fn git(
+    dir: &Path,
+    args: &[&str],
+    auth: Option<&[(String, String)]>,
+    rec: &Recorder,
+) -> Result<()> {
+    let mut cmd = Command::new("git");
+    cmd.args(args)
         .current_dir(dir)
         .env("GIT_TERMINAL_PROMPT", "0")
         .env("GIT_CONFIG_NOSYSTEM", "1")
-        .stdin(Stdio::null())
+        .stdin(Stdio::null());
+    for (k, v) in auth.unwrap_or_default() {
+        cmd.env(k, v);
+    }
+    let out = cmd
         .output()
         .await
         .context("running git — is it installed on this node?")?;
@@ -248,6 +274,11 @@ async fn static_build(
         })?;
 
     let out_dir = src.join(rel.trim_matches('/'));
+    contained(
+        &env.work_dir.join(format!("d{}", job.deployment_id)),
+        &out_dir,
+        &rel,
+    )?;
     verify_static(&out_dir, &rel, env.max_artifact_mb)?;
 
     let dest = env
@@ -278,6 +309,26 @@ async fn static_build(
     })
 }
 
+/// The output directory must be a real directory inside this build's workspace.
+/// The repository and every install script control what is in there, so a
+/// symlinked `dist` (or root directory) would otherwise make `place` move or
+/// publish a directory from anywhere on the host.
+fn contained(work: &Path, out_dir: &Path, rel: &str) -> Result<()> {
+    if std::fs::symlink_metadata(out_dir)
+        .map(|m| m.file_type().is_symlink())
+        .unwrap_or(false)
+    {
+        bail!("output directory `{rel}` is a symlink; it must be a real directory");
+    }
+    let (Ok(work), Ok(out)) = (std::fs::canonicalize(work), std::fs::canonicalize(out_dir)) else {
+        return Ok(()); // does not exist: verify_static says so
+    };
+    if !out.starts_with(&work) {
+        bail!("output directory `{rel}` resolves outside the build workspace");
+    }
+    Ok(())
+}
+
 /// The checks that catch the mistake everybody makes: a build that "succeeded"
 /// into the wrong directory. Publishing that produces a live site serving
 /// nothing, with a green tick next to it.
@@ -586,6 +637,25 @@ mod tests {
         d
     }
 
+    #[test]
+    fn a_symlinked_or_escaping_output_directory_is_refused() {
+        let outside = dir_with(&[("index.html", "<h1>host</h1>")]);
+        let work = dir_with(&[("real/index.html", "<h1>ok</h1>")]);
+        std::os::unix::fs::symlink(outside.path(), work.path().join("dist")).unwrap();
+        std::os::unix::fs::symlink(outside.path(), work.path().join("root")).unwrap();
+
+        assert!(contained(work.path(), &work.path().join("real"), "real").is_ok());
+        let err = contained(work.path(), &work.path().join("dist"), "dist")
+            .unwrap_err()
+            .to_string();
+        assert!(err.contains("symlink"), "{err}");
+        // A symlinked root directory with a real-looking output under it.
+        let err = contained(work.path(), &work.path().join("root/."), "root/.")
+            .unwrap_err()
+            .to_string();
+        assert!(err.contains("outside"), "{err}");
+    }
+
     #[test]
     fn verification_rejects_a_missing_output_directory() {
         let d = tempfile::tempdir().unwrap();