fix: register the forward add local-port flag

`sshkeeper forward add` could never succeed. RunE read --local-port and init()
marked it required, but the flag was never registered on forwardAddCmd. Cobra
ignores MarkFlagRequired for an unknown flag, and GetInt returns 0 for one, so
every invocation failed validation with "invalid local port 0: must be
1-65535". There was no argument combination that worked.

Register the flag so both the read and the required annotation bind to a real
option. README and the guide already documented --local-port, so the intent was
there from the start; only the registration was missing.

The existing tests missed this because they build a throwaway cobra.Command,
register the flags on it themselves, and pass it to forwardAddCmd.RunE — the
real command's flag set was never exercised. Add a test that parses argv into
forwardAddCmd's own flags, plus one pinning the required annotation. Both fail
against the unfixed command with "unknown flag: --local-port".

Present since c2edaa4, so v0.2.0 and v0.3.0 both ship it broken.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
mirivlad 2026-08-18 18:46:08 +08:00
parent 44156a11af
commit cc83802244
2 changed files with 82 additions and 0 deletions

View File

@ -199,6 +199,7 @@ func init() {
forwardAddCmd.Flags().String("name", "", "Forward name")
forwardAddCmd.Flags().String("description", "", "Forward description")
forwardAddCmd.Flags().String("local-addr", "127.0.0.1", "Listen address")
forwardAddCmd.Flags().Int("local-port", 0, "Listen port")
forwardAddCmd.MarkFlagRequired("local-port")
forwardAddCmd.Flags().String("remote-addr", "", "Target address")
forwardAddCmd.Flags().Int("remote-port", 0, "Target port")

View File

@ -7,8 +7,22 @@ import (
"github.com/mirivlad/sshkeeper/internal/db"
"github.com/mirivlad/sshkeeper/internal/model"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
)
// restoreFlags returns every flag touched during the test back to its default.
// The cobra commands are package-level singletons, so parsing argv into one
// leaks state into whatever test runs next.
func restoreFlags(t *testing.T, cmd *cobra.Command) {
t.Helper()
t.Cleanup(func() {
cmd.Flags().Visit(func(f *pflag.Flag) {
_ = f.Value.Set(f.DefValue)
f.Changed = false
})
})
}
func TestForwardEditUpdatesEnabledFlag(t *testing.T) {
testDB, err := db.Open(t.TempDir())
if err != nil {
@ -110,3 +124,70 @@ func TestForwardAddStoresNameAndDescription(t *testing.T) {
t.Fatalf("unexpected forward metadata: %#v", forwards[0])
}
}
// TestForwardAddParsesItsOwnFlags drives the real forwardAddCmd flag set the
// way the CLI does, instead of handing RunE a command built by the test.
//
// Regression: RunE read --local-port and init() marked it required, but the
// flag was never registered on forwardAddCmd. Cobra silently ignores
// MarkFlagRequired for an unknown flag and GetInt returns 0 for one, so every
// real invocation died on "invalid local port 0" while the sibling tests --
// which registered the flag on a throwaway command themselves -- kept passing.
func TestForwardAddParsesItsOwnFlags(t *testing.T) {
testDB, err := db.Open(t.TempDir())
if err != nil {
t.Fatalf("open db: %v", err)
}
defer testDB.Close()
previousDB := appDB
appDB = testDB
t.Cleanup(func() { appDB = previousDB })
server := &model.Server{Alias: "web", Host: "web.example.org", Port: 22, User: "root", AuthMethod: model.AuthKey}
if err := appDB.CreateServer(server); err != nil {
t.Fatalf("create server: %v", err)
}
restoreFlags(t, forwardAddCmd)
if err := forwardAddCmd.Flags().Parse([]string{
"--name", "Local PostgreSQL",
"--type", "local",
"--local-port", "15432",
"--remote-addr", "db01.internal.example.com",
"--remote-port", "5432",
}); err != nil {
t.Fatalf("parse forward add flags: %v", err)
}
if err := forwardAddCmd.RunE(forwardAddCmd, []string{"web"}); err != nil {
t.Fatalf("add forward: %v", err)
}
forwards, err := appDB.GetForwards(server.ID)
if err != nil {
t.Fatalf("get forwards: %v", err)
}
if len(forwards) != 1 {
t.Fatalf("expected one forward, got %d", len(forwards))
}
got := forwards[0]
if got.LocalPort != 15432 {
t.Fatalf("local port not carried through: got %d, want 15432", got.LocalPort)
}
if got.Type != model.ForwardLocal || got.RemoteAddr != "db01.internal.example.com" || got.RemotePort != 5432 {
t.Fatalf("unexpected forward: %#v", got)
}
}
// TestForwardAddRequiresLocalPort pins the flag's registration and its required
// annotation, which is what MarkFlagRequired silently failed to attach.
func TestForwardAddRequiresLocalPort(t *testing.T) {
flag := forwardAddCmd.Flags().Lookup("local-port")
if flag == nil {
t.Fatal("forward add does not register --local-port")
}
if _, ok := flag.Annotations[cobra.BashCompOneRequiredFlag]; !ok {
t.Fatal("--local-port is registered but not marked required")
}
}