feat(push): force-lease gate for bare force-push to feature branches (embedder P0, #2677)

crates/agentic-git/src/main.rs

@@ -375,6 +375,24 @@ fn shim_main() {
                 );
                 std::process::exit(1);
             }
+            // #2677 (embedder P0): a BARE force-push (`--force`/`-f`/`+refspec`) to a
+            // NON-protected branch can silently overwrite another agent's/session's
+            // commits — `push_protected_violation` above ignores force ("HOW not WHAT"),
+            // so it never catches this. Require a lease (footgun-removal, not
+            // capability-removal); pure deletions are exempt (ALL-not-ANY, #2677 F1).
+            // Runs AFTER the protected guard so a protected-ref force is denied there.
+            if let Some(reason) = push_force_without_lease_violation(&args) {
+                emit_deny_error(subcommand, &reason, &agent, Some(&binding));
+                write_git_event_typed(
+                    &home,
+                    &agent,
+                    subcommand,
+                    "deny_force_no_lease",
+                    None,
+                    Some(&reason),
+                );
+                std::process::exit(1);
+            }
             // #4 Δa v5: the snapshot namespace must be shim-unpushable — a
             // "private" ref namespace is not private to `git push`. Two
             // cheap layers (text substring + resolved commit-tip match);
@@ -2338,6 +2356,7 @@ fn current_branch_of(worktree: &str) -> String {
 struct PushArgv {
     bulk: bool,
     delete: bool,
+    force: bool,
     positionals: Vec<String>,
 }
 
@@ -2348,6 +2367,19 @@ fn push_opt_takes_value(opt: &str) -> bool {
     matches!(opt, "-o" | "--push-option" | "--receive-pack" | "--exec")
 }
 
+/// A value-taking SHORT option in ATTACHED (glued-value) form, e.g. `-o+force`
+/// = `-o` with push-option value `+force`. Git consumes the REST of the `-o…`
+/// token as the option's value, so it is NEVER a `-f` force cluster (reviewer4 F1:
+/// `git push -o+force` reaches remote push-option handling, not force). `-o` is the
+/// only short value-option `git push` accepts; the separate form `-o <val>` is
+/// handled by `push_opt_takes_value`, and long options use `--opt[=val]` (a `--`
+/// token, already excluded from force/positional classification). This token must
+/// be consumed WHOLE so its value (which may contain `f` or a leading `+`) is never
+/// read as force/delete.
+fn is_attached_short_value_opt(arg: &str) -> bool {
+    arg.len() > 2 && arg.starts_with("-o")
+}
+
 /// `--delete` / `-d` (or an unambiguous long-prefix of `--delete`; `--d`/`--de`
 /// collide with `--dry-run`, so require ≥3 chars). This flag turns the push
 /// positionals into refs to DELETE rather than refspecs.
@@ -2356,14 +2388,32 @@ fn is_delete_push_flag(arg: &str) -> bool {
         || matches!(arg.strip_prefix("--"), Some(n) if n.len() >= 3 && "delete".starts_with(n))
 }
 
-/// Option-aware parse of a `push` argv into (bulk?, delete?, positionals) — skips
+/// A BARE (unconditional) force FLAG: `--force` (exact) or a single-dash short
+/// cluster containing `f` (`-f`, `-uf`, `-fu`). `--force-with-lease[=…]` and
+/// `--force-if-includes` are the SAFE lease forms — they refuse the push if the
+/// remote moved — and are deliberately NOT matched. `--force` is an exact match:
+/// git rejects the ambiguous abbreviation `--forc` (shared with
+/// `--force-with-lease`), so no long-form abbreviation handling is needed. The
+/// `+refspec` positional force form is detected in `parse_push_argv`, not here.
+///
+/// The `contains('f')` short-cluster test is safe ONLY because the caller
+/// (`parse_push_argv`) first skips the attached push-option token `-o<val>`
+/// (`is_attached_short_value_opt`) — otherwise `-o+force`'s value would misfire it
+/// (reviewer4 F1). `-f` is the sole `f`-bearing short flag `git push` has, so any
+/// remaining single-dash-with-`f` token is a genuine force cluster.
+fn is_bare_force_flag(arg: &str) -> bool {
+    arg == "--force" || (arg.starts_with('-') && !arg.starts_with("--") && arg.contains('f'))
+}
+
+/// Option-aware parse of a `push` argv into (bulk?, delete?, force?, positionals) — skips
 /// flags and the values of value-taking options, so a positional is genuinely a
 /// remote/ref (not e.g. a `-o` push-option value). fugu design review: a bare
 /// positional-count heuristic mis-reads `--delete <ref>` and option values.
 fn parse_push_argv(args: &[String]) -> PushArgv {
     let mut p = PushArgv {
         bulk: false,
         delete: false,
+        force: false,
         positionals: Vec::new(),
     };
     let mut i = 1; // skip "push"
@@ -2373,16 +2423,32 @@ fn parse_push_argv(args: &[String]) -> PushArgv {
             i += 2; // the option AND its value
             continue;
         }
+        // Attached glued-value short option (`-o<val>`, e.g. `-o+force`): ONE token —
+        // skip it (never a force cluster; its value may contain `f`/`+`). reviewer4 F1.
+        if is_attached_short_value_opt(a) {
+            i += 1;
+            continue;
+        }
         if a.starts_with('-') && a != "-" {
             if is_bulk_push_flag(a) {
                 p.bulk = true;
             }
             if is_delete_push_flag(a) {
                 p.delete = true;
             }
+            if is_bare_force_flag(a) {
+                p.force = true;
+            }
             i += 1;
             continue;
         }
+        // A `+`-prefixed positional IS a force refspec (`+src:dst`) — bare force via
+        // the refspec form rather than a `--force`/`-f` flag. Detected here (not in
+        // `is_bare_force_flag`) so the option-aware skip above never mistakes a
+        // `-o +val` push-option VALUE for a force refspec.
+        if a.starts_with('+') {
+            p.force = true;
+        }
         p.positionals.push(a.clone());
         i += 1;
     }
@@ -2510,6 +2576,90 @@ fn push_cross_branch_violation(
     None
 }
 
+/// #t-…93550-2 (embedder P0, ports agend-terminal #2677): a BARE force-push
+/// (`--force`/`-f`/`+refspec`) to a NON-protected branch can SILENTLY OVERWRITE
+/// commits already on the remote branch — another agent's or session's work, or a
+/// wrong-based branch. `push_protected_violation` deliberately ignores force ("HOW
+/// not WHAT") and only guards protected refs, so it never catches this. Require a
+/// LEASE (`--force-with-lease`/`--force-if-includes`) instead: it refuses the push
+/// if the remote moved since the pusher's last fetch, REMOVING the footgun while
+/// KEEPING the legitimate rebase-then-force workflow (footgun-removal, not
+/// capability-removal). Returns an actionable deny reason (with an executable retry
+/// sequence), or `None` to allow.
+///
+/// Runs AFTER `push_protected_violation` in the push arm, so a force-push to a
+/// protected ref is already denied there. Deletions don't overwrite history →
+/// exempt (`is_pure_delete_push`). NB: git makes a trailing `--force` override a
+/// `--force-with-lease`, so ANY bare force present = unconditional and is denied
+/// even if a lease flag co-occurs (`p.force` is set by any bare `--force`/`-f`/`+`).
+fn push_force_without_lease_violation(args: &[String]) -> Option<String> {
+    let p = parse_push_argv(args);
+    if !p.force || is_pure_delete_push(&p) {
+        return None;
+    }
+    let (remote, branch) = force_push_target(&p);
+    let seq = match (remote.as_deref(), branch.as_deref()) {
+        (Some(r), Some(b)) => format!("git fetch {r} {b} && git push --force-with-lease {r} {b}"),
+        _ => "git fetch <remote> <branch> && git push --force-with-lease <remote> <branch>"
+            .to_string(),
+    };
+    Some(format!(
+        "bare force-push denied: `--force` / `-f` / a `+refspec` can SILENTLY OVERWRITE \
+         commits already on the remote branch (another agent's or session's work). Re-run \
+         with a lease — it refuses the push if the remote moved since your last fetch, so you \
+         cannot clobber commits you have not seen:\n  {seq}\nProtected refs stay hard-denied \
+         regardless; this guards feature branches. If you genuinely intend to discard remote \
+         commits, fetch first so the lease baseline is current."
+    ))
+}
+
+/// A PURE deletion push removes refs rather than overwriting history → exempt from
+/// the force gate. `--delete`/`-d` (`p.delete`) makes EVERY named ref a deletion, so
+/// it is unconditionally exempt. Otherwise a push is a pure deletion only when it
+/// names a remote + ≥1 refspec and EVERY refspec is a `:<dest>` colon-deletion (after
+/// an optional `+`).
+///
+/// #2677 F1 (agend-terminal, CONFIRMED bypass): this MUST be ALL-not-ANY. A mixed
+/// `git push --force origin :del real` deletes `:del` AND force-overwrites `real`; an
+/// any-refspec exemption would let `real`'s force through. Exempting only when ALL
+/// refspecs are deletions keeps a lone `:del` (or `--delete`) exempt while a mixed
+/// push stays gated.
+///
+/// Deliberate fail-CLOSED over-deny (safe — a footgun guard may over-deny when a
+/// clean alternative exists): a CLUSTERED `-df` (force+delete of a non-colon ref)
+/// is denied rather than exempted, because `is_delete_push_flag` matches only the
+/// standalone `-d`/`--delete`, not a `-d` glued into a short cluster. The user can
+/// re-run as `git push --delete …` (exempt) or `--force-with-lease`. Never a
+/// fail-open: the clustered form is denied, not allowed.
+fn is_pure_delete_push(p: &PushArgv) -> bool {
+    if p.delete {
+        return true;
+    }
+    p.positionals.len() > 1
+        && p.positionals[1..]
+            .iter()
+            .all(|a| a.strip_prefix('+').unwrap_or(a).starts_with(':'))
+}
+
+/// Best-effort `(remote, branch)` for the deny message's retry sequence — only when
+/// BOTH are explicitly on the command line (≥2 positionals: a remote followed by a
+/// refspec). With fewer we cannot tell a remote from a refspec, so we return
+/// `(None, None)` and the caller falls back to the generic `<remote> <branch>`
+/// template. Reuses the option-aware `PushArgv.positionals`, so a `-o <val>` value is
+/// never mistaken for a remote/refspec.
+fn force_push_target(p: &PushArgv) -> (Option<String>, Option<String>) {
+    if p.positionals.len() < 2 {
+        return (None, None);
+    }
+    let remote = Some(p.positionals[0].trim_start_matches('+').to_string());
+    let branch = p.positionals.get(1).map(|a| {
+        let a = a.strip_prefix('+').unwrap_or(a);
+        let dest = a.rsplit(':').next().unwrap_or(a);
+        dest.strip_prefix("refs/heads/").unwrap_or(dest).to_string()
+    });
+    (remote, branch)
+}
+
 // ── Exec ────────────────────────────────────────────────────────────────
 
 fn exec_with_conflict_guidance(

crates/agentic-git/src/tests.rs

@@ -3195,3 +3195,165 @@ fn cross_branch_push_allowed_forms() {
     // an unbound caller (empty assigned) is never restricted here
     assert!(push_cross_branch_violation(&vargs(&["push", "origin", "feat/b"]), "", "feat/a", false).is_none());
 }
+
+// ── #2677 embedder P0: bare force-push requires a lease (feature branches). ──
+
+#[test]
+fn push_force_without_lease_denies_bare_force_to_feature_branch_2677() {
+    // THE core #2677 gate: a bare `--force`/`-f`/`+refspec` to a non-protected
+    // branch is denied — it could silently clobber remote commits. RM: drop the
+    // `!p.force` guard (or stop setting `p.force`) → these go RED.
+    let deny = |a: &[&str]| push_force_without_lease_violation(&vargs(a));
+    for a in [
+        &["push", "--force", "origin", "feat/x"][..],
+        &["push", "-f", "origin", "feat/x"][..],
+        &["push", "-uf", "origin", "feat/x"][..], // bundled short cluster
+        &["push", "-fu", "origin", "feat/x"][..],
+        &["push", "origin", "+feat/x"][..], // +refspec force form
+        &["push", "origin", "+HEAD:feat/x"][..], // +src:dst force form
+        &["push", "--force", "feat/x"][..], // no explicit remote positional
+        &["push", "origin", "--", "+feat/x"][..], // + refspec after end-of-options
+    ] {
+        assert!(deny(a).is_some(), "bare force must be DENIED: {a:?}");
+    }
+    // the deny message carries the executable lease retry sequence.
+    let msg = deny(&["push", "--force", "origin", "feat/x"]).unwrap();
+    assert!(
+        msg.contains("git fetch origin feat/x")
+            && msg.contains("git push --force-with-lease origin feat/x"),
+        "actionable retry seq: {msg}"
+    );
+    // with too few positionals to name (remote, branch), fall back to the generic
+    // template rather than mis-labelling a lone refspec as the remote.
+    assert!(deny(&["push", "--force", "feat/x"])
+        .unwrap()
+        .contains("git push --force-with-lease <remote> <branch>"));
+}
+
+#[test]
+fn push_force_without_lease_allows_lease_and_normal_forms_2677() {
+    // Lease forms are the SAFE way to force → allowed. Normal (non-force) pushes are
+    // untouched (zero regression to legitimate pushes).
+    let allow = |a: &[&str]| push_force_without_lease_violation(&vargs(a));
+    for a in [
+        &["push", "--force-with-lease", "origin", "feat/x"][..],
+        &["push", "--force-with-lease=origin/feat/x", "origin", "feat/x"][..],
+        &["push", "--force-if-includes", "--force-with-lease", "origin", "feat/x"][..],
+        &["push", "origin", "feat/x"][..],
+        &["push", "-u", "origin", "feat/x"][..],
+        &["push"][..],
+        &["push", "origin"][..],
+        &["push", "-o", "ci.skip", "origin", "feat/x"][..], // push-option value not mis-read
+        &["push", "-o", "+val", "origin", "feat/x"][..],    // + inside an option value ≠ force
+    ] {
+        assert!(allow(a).is_none(), "must be ALLOWED: {a:?} -> {:?}", allow(a));
+    }
+}
+
+#[test]
+fn push_force_trailing_bare_force_overrides_lease_2677() {
+    // git makes a trailing `--force` override a `--force-with-lease`, so a bare
+    // `--force` present alongside a lease flag is still unconditional → denied.
+    assert!(push_force_without_lease_violation(&vargs(&[
+        "push", "--force-with-lease", "--force", "origin", "feat/x"
+    ]))
+    .is_some());
+}
+
+#[test]
+fn push_force_pure_deletion_is_exempt_2677() {
+    // Deletions don't overwrite history → exempt from the force gate even with force.
+    let allow = |a: &[&str]| push_force_without_lease_violation(&vargs(a));
+    for a in [
+        &["push", "--force", "origin", ":del"][..], // colon-delete
+        &["push", "--force", "origin", ":a", ":b"][..], // ALL refspecs are deletes
+        &["push", "--force", "origin", "+:del"][..], // force-delete refspec
+        &["push", "--delete", "origin", "foo"][..], // --delete flag (no force needed)
+        &["push", "--force", "--delete", "origin", "foo"][..], // force + --delete
+    ] {
+        assert!(allow(a).is_none(), "pure deletion must be EXEMPT: {a:?} -> {:?}", allow(a));
+    }
+}
+
+#[test]
+fn push_force_mixed_delete_and_overwrite_stays_gated_2677_f1() {
+    // #2677 F1 (CONFIRMED bypass in agend-terminal): a mixed push that deletes one
+    // ref AND force-overwrites another must STAY GATED — the deletion must NOT exempt
+    // the whole push (that was the any-arg bug). RED-first: an any-refspec
+    // `is_pure_delete_push` returns true here → these wrongly return None.
+    let deny = |a: &[&str]| push_force_without_lease_violation(&vargs(a));
+    for a in [
+        &["push", "--force", "origin", ":del", "real"][..],  // delete + overwrite
+        &["push", "--force", "origin", "real", ":del"][..],  // order-independent
+        &["push", "--force", "origin", "+:del", "real"][..], // +delete + overwrite
+        &["push", "origin", "+:del", "real"][..],            // force via + on the delete refspec
+    ] {
+        assert!(deny(a).is_some(), "mixed delete+overwrite must stay GATED: {a:?}");
+    }
+}
+
+#[test]
+fn is_pure_delete_push_is_all_not_any_2677_f1() {
+    // Direct unit pin of the ALL-not-ANY discriminator (the exact #2677 F1 fix).
+    let pure = |a: &[&str]| is_pure_delete_push(&parse_push_argv(&vargs(a)));
+    // ALL refspecs are deletions → pure.
+    assert!(pure(&["push", "origin", ":a"]));
+    assert!(pure(&["push", "origin", ":a", ":b"]));
+    assert!(pure(&["push", "origin", "+:a"]));
+    assert!(pure(&["push", "--delete", "origin", "foo"])); // --delete → every ref a deletion
+    // a single non-delete refspec means NOT pure (force on `real` must stay gated).
+    assert!(!pure(&["push", "origin", ":a", "real"]));
+    assert!(!pure(&["push", "origin", "real", ":a"]));
+    assert!(!pure(&["push", "origin", "feat/x"]));
+    assert!(!pure(&["push", "origin"])); // remote only, no refspec → not a deletion
+}
+
+#[test]
+fn is_bare_force_flag_matches_force_forms_not_lease_2677() {
+    for f in ["--force", "-f", "-uf", "-fu"] {
+        assert!(is_bare_force_flag(f), "{f} must be a bare force flag");
+    }
+    for f in [
+        "--force-with-lease",
+        "--force-with-lease=origin/main",
+        "--force-if-includes",
+        "--forc", // ambiguous long abbrev (git rejects) — not matched
+        "-u",
+        "--",
+        "--tags",
+        "--delete",
+    ] {
+        assert!(!is_bare_force_flag(f), "{f} must NOT be a bare force flag");
+    }
+}
+
+#[test]
+fn push_force_option_value_not_misclassified_as_force_2677_f1() {
+    // reviewer4 F1: git accepts the ATTACHED push-option form `-o<value>`; a value
+    // containing `f` (or a leading `+`) is a push-option VALUE, not a force flag.
+    // RED before the fix: `-o+force` fell through to is_bare_force_flag's
+    // `contains('f')` → force=true → the legitimate non-force push was wrongly DENIED.
+    let allow = |a: &[&str]| push_force_without_lease_violation(&vargs(a));
+    for a in [
+        &["push", "-o+force", "origin", "feat/x"][..], // attached, value has a `+` and `f`
+        &["push", "-oforce", "origin", "feat/x"][..],  // attached, value has `f`
+        &["push", "-oci.f", "origin", "feat/x"][..],
+        &["push", "--push-option=+x", "origin", "feat/x"][..], // long attached (already `--`-safe)
+        &["push", "-o", "ci.f", "origin", "feat/x"][..],       // separate form, control
+    ] {
+        assert!(allow(a).is_none(), "push-option value ≠ force: {a:?} -> {:?}", allow(a));
+    }
+    // The fix must NOT open a reverse fail-open — genuine force STAYS denied even
+    // when clustered with other short flags or sitting next to a push-option.
+    let deny = |a: &[&str]| push_force_without_lease_violation(&vargs(a));
+    for a in [
+        &["push", "-f", "origin", "feat/x"][..],
+        &["push", "-uf", "origin", "feat/x"][..],
+        &["push", "-fu", "origin", "feat/x"][..],
+        &["push", "-fo", "pushopt", "origin", "feat/x"][..], // `-f -o pushopt` — force IS present
+        &["push", "-o", "x", "--force", "origin", "feat/x"][..],
+        &["push", "--force", "-oval", "origin", "feat/x"][..],
+    ] {
+        assert!(deny(a).is_some(), "genuine force must STAY denied: {a:?}");
+    }
+}