Skip to content

fleet(proxy ssh): clustered SSH options are not parsed — operand-bearing options inside flag clusters (and -P) break the recovery invocation #6679

Closed

Description

@dezren39

Finding (from PR review)

fleet proxy ssh argument splitting does not parse operand-bearing SSH options inside flag clusters, and omits at least one operand-bearing option (-P), so valid invocations are broken.

Reviewer chatgpt-codex-connector[bot] (Codex, P2) on PR #6403, 2026-11-06T00:49:20Z:
#6403 (comment)

"Parse operand-bearing SSH options inside flag clusters — For valid invocations such as fleet proxy ssh -- -vL 127.0.0.1:2222:localhost:22, the exact-string lookup does not recognize that -vL requires an operand. It consequently inserts user@100.99.113.103 immediately after -vL, where OpenSSH consumes it as the forwarding specification and exits with Bad local forwarding specification. The table also omits -P tag, documented in the OpenSSH manual, causing another operand-boundary error. Parse clustered option characters and account for all operand-bearing options before separating the remote command."

Source PR: #6403 feat(fleet): add proxy ssh recovery verb (open; parent design issue #5697 does not cover this defect — dedupe searched by PR number, split_ssh_options, "operand-bearing", "proxy ssh", "flag cluster": no existing issue covers it).

No-response evidence

  • The comment is now over 24 hours old (grace period for open PRs passed; first flagged by pr-comment-review 2026-11-06 15:54 CST, rechecked this run).
  • No replies in the thread; the review thread is unresolved (GraphQL reviewThreads verified this run).
  • PR head is unchanged since the review: single commit 2817c11837 (2026-11-06T00:42:38Z, before the comment); no fix commit followed.

Still-live evidence (verified on PR head 2817c11837 this run)

pkgs/fleet/crates/fleet/src/main.rs on the PR head, fresh gh api raw read:

  • Line 638: fn split_ssh_options_and_command(...); line 639-642: OPTIONS_WITH_OPERANDS table contains -B -b -c -D -E -e -F -I -i -J -L -l -m -O -o -p -Q -R -S -W -w — no -P.
  • Line 655: if OPTIONS_WITH_OPERANDS.contains(&arg.as_str()) — exact-string match only; a clustered argument such as -vL never matches, so its operand is treated as the start of the remote command and the fixed destination is inserted in the operand's place.

Acceptance criteria

  • Clustered SSH options are parsed character-by-character: an operand-bearing character inside a cluster consumes the rest of the cluster or the next argument as its operand, and the destination is inserted only after the full option prefix.
  • -P (and any other documented operand-bearing ssh options missing from the table) are accounted for.
  • Tests cover at least: -vL <spec> clustered forwarding, -P <tag>, a separated-operand option (existing behavior), and a plain remote command with no options.
  • Or: resolve this issue by a documented decision explaining why clustered forms are intentionally unsupported, with the rejection made explicit (a clear error instead of a malformed ssh invocation).

Activity

  1. added
    kind/bugSomething isn't working (prefixed form of the classic bug label)
    task/triageQueue packet kind: verify, reproduce, and route this item
    priority/normalOrdinary priority (the default)
    queue/queuedHas a queued task in the Supabase agent queue
    on Nov 7, 2026
  2. developing-today commented on Nov 7, 2026

    @developing-today
    Contributor
  3. added
    app/fleetpkgs/fleet multicall binary + platform crates
    on Nov 7, 2026
  4. dezren39 commented on Nov 7, 2026

    @dezren39
    MemberAuthor

    🤖 Agent note · filed

  5. dezren39 commented on Nov 7, 2026

    @dezren39
    MemberAuthor

    Picked up by repo-worker-code-t (Lane T, fallback mode — queue backend down, fleet queue health 503 PostgREST PGRST002). Triage of #6679: revalidating the Codex P2 clustered-options finding against PR #6403 head first.

    🤖 Agent note · picked up

  6. added
    queue/claimedQueue worker lifecycle state
    work/assignedQueue worker lifecycle state
    and removed
    queue/queuedHas a queued task in the Supabase agent queue
    on Nov 7, 2026
  7. 21 remaining items

  8. dezren39 commented on Nov 7, 2026

    @dezren39
    MemberAuthor

    Picked up by repo-worker-code-r (lane R, fallback mode — queue backend down) for the small-fix: parse clustered SSH options character-by-character in split_ssh_options_and_command and add -P to the operand-bearing set, as a new commit on PR #6403's branch queue/10604-5697-proxy-ssh (current head bb71f3d, includes the #6680 fix), per lane T's verified triage route on this issue.

  9. added
    queue/claimedQueue worker lifecycle state
    work/assignedQueue worker lifecycle state
    and removed
    queue/queuedHas a queued task in the Supabase agent queue
    status/triagedTriaged and routed (see agent note)
    on Nov 7, 2026
  10. dezren39 commented on Nov 7, 2026

    @dezren39
    MemberAuthor

    Progress from repo-worker-code-r: implementation_complete in ~/git/fleet/fb-6680: split_ssh_options_and_command now walks each cluster char-by-char (operand char consumes rest-of-cluster or next argument), -P added to the operand-bearing char set; new unit test covers -vL , -P , attached -L, and missing-operand error. Targeted tests pass (unit 4/4, integration 1/1). Next: full fleet suite, commit + push to PR #6403's branch, disposition Codex thread r4487818000.

  11. dezren39 commented on Nov 7, 2026

    @dezren39
    MemberAuthor

    Fixed — commit 201836ee9d pushed to PR #6403's branch queue/10604-5697-proxy-ssh (on top of the #6680 fix bb71f3d97f).

    • split_ssh_options_and_command (pkgs/fleet/crates/fleet/src/main.rs) no longer matches whole arguments against an operand table. It walks each leading-dash argument character-by-character over an operand-bearing char set per ssh(1) that now includes P: an operand-bearing character consumes the rest of its cluster as the operand, or the next argument when it ends the cluster — so the fixed destination is inserted only after the full option prefix.
    • Attached operands (-L<spec>, -o<opt>) and separated operands behave exactly as before; -- and missing-operand errors keep their meanings (messages now name the operand character and cluster).
    • Tests: new proxy_ssh_splits_clustered_options_and_operands covers -vL <spec>, -P <tag>, attached -L<spec>, and the clustered missing-operand error; existing separated-operand and plain-command cases remain covered by the pinned argv tests. Verified on laptop: full fleet crate suite green (17 unit + 1 integration).
    • Codex P2 thread dispositioned in place: reply feat(fleet): add proxy ssh recovery verb #6403 (comment)
    • CI on PR feat(fleet): add proxy ssh recovery verb #6403 for 201836ee9d was still pending (basic checks queued) at close-out; the earlier run for bb71f3d97f was cancelled as superseded by this push, which is expected. PR feat(fleet): add proxy ssh recovery verb #6403 merges under its own lifecycle.

    Worked by repo-worker-code-r (lane R), fallback mode (queue backend down).

  12. dezren39 commented on Nov 7, 2026

    @dezren39
    MemberAuthor

    Released by repo-worker-code-r (outcome: done). Fix landed as commit 201836e on PR #6403's branch queue/10604-5697-proxy-ssh: split_ssh_options_and_command parses clusters char-by-char, -P added to the operand-bearing set; tests cover -vL, -P, attached -L and missing-operand. Full fleet suite green on laptop; Codex P2 thread replied (r4499735056). PR #6403 CI pending at release. Issue #6679 closed as completed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    app/fleetpkgs/fleet multicall binary + platform crateskind/bugSomething isn't working (prefixed form of the classic bug label)priority/normalOrdinary priority (the default)source/pr-comment-reviewFiled by the pr-comment-review jobstatus:donetask/small-fixQueue packet kind: bounded fix with reproduction and targeted tests

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions