From f4645b9408bffb13f9ed7b70f55162024b671edb Mon Sep 17 00:00:00 2001 From: "XXV.CC" Date: Sat, 26 Sep 2026 10:00:02 +0000 Subject: [PATCH] refactor: simplify invite flow and lifecycle result handling Combine the default terminal lifetime prompts and share per-display account observations. Separate invite option handling, expose explicit upgrade outcomes, share timer/deletion steps, and consolidate doctor guidance. Keep existing flags, backends, identity checks, recovery rules, and confirmation gates. Retire the old menu process after a visible upgrade even when durability confirmation fails. Validated with ordinary and race suites as nobody, full root race integration in an isolated container, real systemd/PTY/SSH lifecycle checks, static analysis, script checks, and amd64/arm64 static builds. --- CHANGELOG.md | 15 + README.en.md | 2 + README.md | 2 + docs/operator-guide.en.md | 4 +- docs/operator-guide.md | 4 +- internal/cli/cli_test.go | 25 +- internal/cli/commands.go | 36 ++- internal/cli/invite.go | 309 +++++--------------- internal/cli/invite_lifetime_test.go | 127 ++++++++ internal/cli/invite_options.go | 216 ++++++++++++++ internal/cli/invite_reuse_root_test.go | 6 +- internal/cli/narrow_terminal_test.go | 48 +++ internal/cli/revoke.go | 50 ++-- internal/cli/revoke_test.go | 2 +- internal/cli/selfcmd.go | 21 +- internal/cli/selfcmd_result_test.go | 169 +++++++++++ internal/schedule/valid.go | 31 +- internal/selfmanage/selfmanage.go | 82 +++--- internal/selfmanage/selfmanage_root_test.go | 118 +++++--- internal/selfmanage/selfmanage_test.go | 13 +- 20 files changed, 860 insertions(+), 420 deletions(-) create mode 100644 internal/cli/invite_lifetime_test.go create mode 100644 internal/cli/invite_options.go create mode 100644 internal/cli/selfcmd_result_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c0c247..6e928bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,21 @@ All notable changes to this project are documented here. +## Unreleased + +- Read each account once per status display, sharing that observation between + table and narrow-terminal layouts while refreshing it on the next display. +- Return explicit upgrade outcomes and exit the menu after a visible executable + replacement even if its directory sync fails; retain the failure exit status. +- Combine the default terminal invite's automatic-removal and lifetime questions + into one prompt. Enter keeps 24 hours; `never` explicitly selects permanence. + Preserve existing CLI flags and piped-input behavior; EOF cancels the prompt. +- Separate invite argument parsing and preconditions from planning and execution; + share timer file validation and the final controlled account-deletion steps + without changing backend compatibility, identity policy, or recovery rules. +- Summarize successful dependency checks and show each shared doctor cleanup + suggestion once, retaining individual failure diagnostics and exit statuses. + ## v2.10.7 - 2026-09-26 - Close the remaining audit findings: clean up helper process groups before diff --git a/README.en.md b/README.en.md index ae8b202..3a1716c 100644 --- a/README.en.md +++ b/README.en.md @@ -59,6 +59,8 @@ The quick start already creates the first invite. Later invites can be created w The interactive flow shows the account, host, port, expiry, sudo state, and login verdict, followed by a command that saves the one-time private key. Only the public key is stored on the server. +The terminal asks for the lifetime once: press Enter for the default 24 hours or enter another number of hours. A permanent account requires an explicit `never`. The complete summary is shown for confirmation before creation. + Send the complete bundle through trusted private chat. After saving the key, the collaborator builds the SSH command from the bundle's Host, Port, and User fields, for example: ```bash diff --git a/README.md b/README.md index 9ea842d..0caf87a 100644 --- a/README.md +++ b/README.md @@ -59,6 +59,8 @@ curl -fsSL https://dl.ll.cd/linux-temp-admin/install.sh | /usr/bin/sudo /bin/sh 交互流程会显示账号、Host、端口、有效期、sudo 状态和登录验证结果，并输出一次性的私钥保存命令。服务器只保存公钥，私钥不会落盘。 +终端中的有效期只需选择一次：回车使用默认 24 小时，或输入其他小时数；永久账号需明确输入 `never`。创建前会显示完整摘要并要求确认。 + 把完整邀请包通过可信私聊发给协作者。对方保存私钥后，使用邀请头部的 Host、Port 和 User 登录，例如： ```bash diff --git a/docs/operator-guide.en.md b/docs/operator-guide.en.md index 859cdd6..89e9156 100644 --- a/docs/operator-guide.en.md +++ b/docs/operator-guide.en.md @@ -33,10 +33,12 @@ The interactive flow: 1. chooses a username, using a random suffix by default; 2. detects or asks for the invite host and SSH port; 3. grants sudo by default, with an option for a regular account; -4. asks whether to auto-delete and then asks the lifetime only when enabled; +4. uses one terminal prompt for the lifetime: Enter keeps 24 hours, and an explicit `never` selects a permanent account; 5. shows the complete summary for confirmation; 6. creates the account and grants, creates a task when automatic revocation is enabled, and only then prints the invite credential. +Explicit lifetime and automatic-removal flags keep their existing rules: `--auto-revoke` cannot be changed to permanent at the prompt, and `--no-auto-revoke` skips the lifetime question. Piped input retains the existing y/n automatic-removal choice; `--yes` never prompts. Invalid terminal input is retried, and end-of-input cancels. The final confirmation clearly identifies permanent accounts. + Before creating anything, the tool checks whether the planned credential is compatible with the effective sshd configuration. An unresolved blocker reported by the check refuses creation, and incomplete knowledge is reported as `UNVERIFIED`. "Verified against the effective sshd config" means only that this configuration check completed without a known blocker or unevaluated rule; it is not end-to-end proof of the network, firewall, PAM, SELinux, or running sshd state. Test the invite through the intended connection path before delivery. ### Host detection diff --git a/docs/operator-guide.md b/docs/operator-guide.md index 31edf13..5397488 100644 --- a/docs/operator-guide.md +++ b/docs/operator-guide.md @@ -33,10 +33,12 @@ 1. 选择用户名，默认使用随机后缀； 2. 探测或询问邀请中的 Host 和 SSH 端口； 3. 默认授予 sudo，也可以选择普通账号； -4. 询问是否自动删除，启用时再询问有效期； +4. 在终端中用一次选择设置有效期，回车默认 24 小时，明确输入 `never` 可选永久； 5. 显示完整摘要并确认； 6. 创建账号和授权；启用自动撤销时创建任务，最后才输出邀请凭据。 +已显式指定的有效期和自动删除开关仍按原规则处理；`--auto-revoke` 不接受交互改成永久，`--no-auto-revoke` 跳过有效期询问。非终端输入仍保留原来的 y/n 自动删除选择，`--yes` 模式不会询问。输入错误会重新询问，输入结束会取消；永久选项会在最终确认摘要中明确显示。 + 创建任何内容前，工具会用 sshd 的有效配置检查计划凭据是否兼容。未解决的配置检查阻碍会拒绝创建，无法完整判断时会在邀请中标记 `UNVERIFIED`。显示“已对照 sshd 有效配置验证”只代表这项配置检查完整通过，不是对网络、防火墙、PAM、SELinux 或运行中 sshd 状态的端到端登录证明；交付前仍应沿实际连接路径测试邀请。 ### Host 探测 diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index a5fbfc3..4444240 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -2531,23 +2531,6 @@ func TestPlanLoginNoPasswordFallbackWhenPasswordsOff(t *testing.T) { } } -// TestPromptHours covers the new interactive lifetime prompt: a value is taken, -// a blank keeps the default, and an out-of-range entry is re-asked. -func TestPromptHours(t *testing.T) { - if got := mustHours(t, "48\n", 24); got != 48 { - t.Errorf("hours = %d, want 48", got) - } - if got := mustHours(t, "\n", 24); got != 24 { - t.Errorf("blank hours = %d, want the default 24", got) - } - if got := mustHours(t, "0\n99999999\n72\n", 24); got != 72 { - t.Errorf("hours after invalid entries = %d, want 72", got) - } - if got := mustHours(t, "", 24); got != 24 { // EOF settles on the default, never loops - t.Errorf("EOF hours = %d, want 24", got) - } -} - func TestPromptYesNoRejectsTypos(t *testing.T) { a, _, errb := newTestApp(t, "never\nmaybe\nn\n") a.StdinIsTTY = func() bool { return true } @@ -2636,12 +2619,6 @@ func TestClassifyRegisteredAccountIdentityStates(t *testing.T) { } } -func mustHours(t *testing.T, in string, def int) int { - t.Helper() - a, _, _ := newTestApp(t, in) - return a.promptHours(def) -} - // TestPlanDepsRefusesBeforeSummaryAndInstallsAfter is a lightweight check that the // dependency split reports missing deps read-only. With no package manager the // plan must refuse (returns false), never claiming an install it cannot do. @@ -2726,7 +2703,7 @@ func TestGeneratedInviteReachesDependencyGateWithoutID(t *testing.T) { } // TestInviteSkipsHoursPromptOnNonTTYStdin is the regression guard for the -// promptHours infinite-loop. promptHours re-asks on invalid input, so on a +// lifetime prompt infinite-loop. The lifetime prompt re-asks on invalid input, so on a // non-TTY stdin feeding non-numeric lines (the `yes n | lta invite` idiom, whose // stream never blanks) it would spin forever. The hours prompt is therefore gated // on StdinIsTTY. This asserts the gate directly — the lifetime question must never diff --git a/internal/cli/commands.go b/internal/cli/commands.go index 8a1cd9a..cd0e0ab 100644 --- a/internal/cli/commands.go +++ b/internal/cli/commands.go @@ -143,7 +143,7 @@ func (a *App) status(args []string) int { // mechanically derived from the username, and would double the table's width to // tell the reader something they already know; `status --user ` still // prints it for the one account being examined. -func (a *App) usersTable(recs []registry.Record, numbered bool) *table.Table { +func (a *App) usersTable(rows [][]string, numbered bool) *table.Table { headers := []string{ a.P.M("用户", "USER"), a.P.M("状态", "STATE"), @@ -157,8 +157,7 @@ func (a *App) usersTable(recs []registry.Record, numbered bool) *table.Table { headers = append([]string{"#"}, headers...) } t := table.New(headers...) - for i, r := range recs { - cells := a.userCells(r) + for i, cells := range rows { if numbered { cells = append([]string{strconv.Itoa(i + 1)}, cells...) } @@ -211,7 +210,13 @@ func (a *App) userCells(r registry.Record) []string { // usersView keeps the comparison table on ordinary terminals and switches to a // vertical record view when the table would be wider than the actual terminal. func (a *App) usersView(recs []registry.Record, numbered bool) string { - full := a.usersTable(recs, numbered).String() + // Use one observation per account for this render. A later refresh or a + // mutating command must obtain its own current account state. + rows := make([][]string, len(recs)) + for i, rec := range recs { + rows[i] = a.userCells(rec) + } + full := a.usersTable(rows, numbered).String() width := 0 if a.TerminalWidth != nil { width = a.TerminalWidth() @@ -229,8 +234,7 @@ func (a *App) usersView(recs []registry.Record, numbered bool) string { a.P.M("端口", "port"), } var out strings.Builder - for i, rec := range recs { - cells := a.userCells(rec) + for i, cells := range rows { prefix := "- " if numbered { prefix = fmt.Sprintf("%d) ", i+1) @@ -929,9 +933,10 @@ func (a *App) doctorBaseEnvironment() doctorResult { } else { a.success(a.P.M("pidfd 进程撤销能力可用。", "pidfd process revocation is available.")) } + var presentDeps []string for _, d := range sysinfo.RequiredDeps(true, true) { if d.Present { - a.success(a.P.M("依赖存在：", "dependency found: ") + d.Label) + presentDeps = append(presentDeps, d.Label) } else { a.warnf("%s%s", a.P.M("缺少依赖：", "missing dependency: "), d.Label) if doctorDependencyIsFatal(d.Label) { @@ -939,6 +944,9 @@ func (a *App) doctorBaseEnvironment() doctorResult { } } } + if len(presentDeps) > 0 { + a.success(a.P.M("依赖检查通过：", "dependencies found: ") + strings.Join(presentDeps, ", ")) + } a.info(a.P.M("包管理器：", "package manager: ") + orNone(sysinfo.PackageManager())) a.info(a.P.M("init 系统：", "init system: ") + sysinfo.InitSystem()) sshPort, sshPortErr := a.detectSSHPort() @@ -1154,6 +1162,7 @@ func (a *App) doctorSudoersDirectory() string { func (a *App) doctorOrphanedArtifacts() doctorResult { var result doctorResult + needsCompact := false // An sshd exception that outlived its account is a standing loosening of the // host's policy, and it re-arms the moment the username is reused. if a.SSHD != nil { @@ -1165,8 +1174,7 @@ func (a *App) doctorOrphanedArtifacts() doctorResult { a.warnf("%s%s", a.P.M("孤儿 sshd 例外（账号不存在或身份无法验证）：", "orphaned sshd exception (the account is absent or its identity is unverified): "), a.SSHD.FilePath(u)) } - a.warnf("%s", a.P.M("请用 `linux-temp-admin cleanup-expired --compact` 清理。", - "remove them with `linux-temp-admin cleanup-expired --compact`.")) + needsCompact = true result.fail() } } @@ -1188,8 +1196,7 @@ func (a *App) doctorOrphanedArtifacts() doctorResult { a.warnf("%s%s", a.P.M("孤儿 sudo 授权（账号不存在或身份无法验证，NOPASSWD:ALL 仍在）：", "orphaned sudo grant (the account is absent or its identity is unverified; NOPASSWD:ALL is still on disk): "), a.Sudoers.FilePath(u)) } - a.warnf("%s", a.P.M("请用 `linux-temp-admin cleanup-expired --compact` 清理。", - "remove them with `linux-temp-admin cleanup-expired --compact`.")) + needsCompact = true result.fail() } } @@ -1204,11 +1211,14 @@ func (a *App) doctorOrphanedArtifacts() doctorResult { a.warnf("%s%s", a.P.M("孤儿自动删除任务（账号不存在或身份无法验证）：", "orphaned auto-delete task (the account is absent or its identity is unverified): "), u) } - a.warnf("%s", a.P.M("请用 `linux-temp-admin cleanup-expired --compact` 清理。", - "remove them with `linux-temp-admin cleanup-expired --compact`.")) + needsCompact = true result.fail() } } + if needsCompact { + a.warnf("%s", a.P.M("请用 `linux-temp-admin cleanup-expired --compact` 清理。", + "remove them with `linux-temp-admin cleanup-expired --compact`.")) + } return result } diff --git a/internal/cli/invite.go b/internal/cli/invite.go index e4564d1..202c3a1 100644 --- a/internal/cli/invite.go +++ b/internal/cli/invite.go @@ -3,7 +3,6 @@ package cli import ( "bytes" "errors" - "flag" "fmt" "os" "strconv" @@ -28,210 +27,32 @@ func (a *App) invite(args []string) int { if !a.requireRoot() { return 1 } - fs := flag.NewFlagSet("invite", flag.ContinueOnError) - fs.SetOutput(a.Err) - prefix := fs.String("prefix", config.DefaultPrefix, "") - userFlag := fs.String("user", "", "") - hostFlag := fs.String("host", "", "") - portFlag := fs.Int("port", 0, "") - hoursFlag := fs.Int("hours", config.DefaultExpireHours, "") - confirmSudo := fs.String("confirm-sudo", "", "") - var fSudo, fNoSudo, fNopasswd, fAuto, fNoAuto, fYes, fAllowNonTTY, fInstallDeps, fNoInstallDeps bool - var fFixSSHD, fNoFixSSHD, fPasswordLogin bool - fs.BoolVar(&fSudo, "sudo", false, "") - fs.BoolVar(&fNoSudo, "no-sudo", false, "") - fs.BoolVar(&fNopasswd, "nopasswd-sudo", false, "") // deprecated alias of --sudo - fs.BoolVar(&fAuto, "auto-revoke", false, "") - fs.BoolVar(&fNoAuto, "no-auto-revoke", false, "") - fs.BoolVar(&fYes, "yes", false, "") - fs.BoolVar(&fYes, "y", false, "") - fs.BoolVar(&fAllowNonTTY, "allow-non-tty-private-key-output", false, "") - fs.BoolVar(&fInstallDeps, "install-deps", false, "") - fs.BoolVar(&fNoInstallDeps, "no-install-deps", false, "") - fs.BoolVar(&fFixSSHD, "fix-sshd", false, "") - fs.BoolVar(&fNoFixSSHD, "no-fix-sshd", false, "") - fs.BoolVar(&fPasswordLogin, "password-login", false, "") - if err := fs.Parse(args); err != nil { - return 1 - } - if fs.NArg() > 0 { - a.errorf("%s %v", a.P.M("未知参数：", "unexpected arguments:"), fs.Args()) - return 1 - } - if (fSudo || fNopasswd) && fNoSudo { - a.errorf("%s", a.P.M("--sudo/--nopasswd-sudo 与 --no-sudo 互斥", - "--sudo/--nopasswd-sudo and --no-sudo are mutually exclusive")) - return 1 - } - if fAuto && fNoAuto { - a.errorf("%s", a.P.M("--auto-revoke 与 --no-auto-revoke 互斥", - "--auto-revoke and --no-auto-revoke are mutually exclusive")) - return 1 - } - if fInstallDeps && fNoInstallDeps { - a.errorf("%s", a.P.M("--install-deps 与 --no-install-deps 互斥", - "--install-deps and --no-install-deps are mutually exclusive")) - return 1 - } - if fFixSSHD && fNoFixSSHD { - a.errorf("%s", a.P.M("--fix-sshd 与 --no-fix-sshd 互斥", "--fix-sshd and --no-fix-sshd are mutually exclusive")) - return 1 - } - if fNopasswd { - fSudo = true - } - if fPasswordLogin && fFixSSHD { - a.errorf("%s", a.P.M("--password-login 与 --fix-sshd 互斥：密码登录的前提正是不改动 sshd", - "--password-login and --fix-sshd are mutually exclusive: password login exists precisely to leave sshd alone")) - return 1 - } - portSet, hoursSet := false, false - fs.Visit(func(fl *flag.Flag) { - switch fl.Name { - case "port": - portSet = true - case "hours": - hoursSet = true - } - }) - - hours := *hoursFlag - if !validate.Hours(hours) { - a.errorf("%s", a.P.M(fmt.Sprintf("--hours 必须在 1..%d 之间", config.MaxExpireHours), - fmt.Sprintf("--hours must be between 1 and %d", config.MaxExpireHours))) - return 1 - } - if !validate.Prefix(*prefix) { - a.errorf("%s", a.P.M("用户名前缀不合法："+*prefix, "invalid username prefix: "+*prefix)) - return 1 - } - username := *userFlag - generatedUsername := username == "" - if username == "" { - // Only the generation path uses the prefix. A prefix in the reserved - // "systemd-" namespace would generate usernames the revoke path refuses to - // delete (user.IsReservedName), so reject it here before generating. An - // explicit --user does not use the prefix and is validated on its own below. - if user.IsReservedName(*prefix + "-") { - a.errorf("%s", a.P.M("用户名前缀落入受保护命名空间（如 systemd-），会创建无法撤销的账号："+*prefix, - "username prefix is in a reserved namespace (e.g. systemd-) and would create an unrevocable account: "+*prefix)) - return 1 - } - // Fill the username's remaining Linux-compatible length with entropy. Even - // the longest accepted prefix retains the historical 40-bit minimum, while - // the default prefix receives 104 bits. - suffixBytes := (31 - len(*prefix)) / 2 - for attempt := 0; attempt < 20; attempt++ { - h, err := a.RandHex(suffixBytes) - if err != nil { - a.errorf("rand: %v", err) - return 1 - } - cand := *prefix + "-" + h - // Dependency planning happens later and may need to install `id`. Use the - // local database while choosing a candidate, then perform the authoritative - // local+NSS check inside the lifecycle lock immediately before creation. - exists, lookupErr := user.Exists(cand) - if lookupErr != nil { - a.errorf("%s: %v", a.P.M("读取账号数据库失败", "reading account database failed"), lookupErr) - return 1 - } - if !exists { - username = cand - break - } - } - if username == "" { - a.errorf("%s", a.P.M("随机用户名多次冲突，请指定 --user", "random username collided repeatedly; specify --user")) - return 1 - } - } - if !validate.Username(username) { - a.errorf("%s", a.P.M("用户名不合法："+username, "invalid username: "+username)) + opts, ok := a.parseInviteOptions(args) + if !ok { return 1 } - // Refuse a reserved/system name (root, daemon, systemd-*, ...): the revoke path - // protects these, so creating one would leave an account the tool can never - // delete — manually or via the auto-revoke timer. This is the authoritative - // gate; it also covers an explicit --user that bypasses the prefix path above. - if user.IsReservedName(username) { - a.errorf("%s", a.P.M("用户名落入受保护/系统命名空间，拒绝创建（撤销将无法删除）："+username, - "username is a reserved/system name and cannot be created (revoke would refuse to delete it): "+username)) + username, generatedUsername, ok := a.resolveInviteUsername(opts.username, opts.prefix) + if !ok { return 1 } - grantSudo := triState(fSudo, fNoSudo) - autoRev := triState(fAuto, fNoAuto) + grantSudo := opts.sudo - // Refuse a non-TTY stdout up front — before any prompt or host probe — so a - // piped run fails immediately rather than after the operator answers. - if !a.StdoutIsTTY() && !fAllowNonTTY { - a.errorf("%s", a.P.M("stdout 非 TTY，拒绝输出一次性私钥/密码（可加 --allow-non-tty-private-key-output）", - "stdout is not a TTY; refusing to print the one-time private key or password (add --allow-non-tty-private-key-output)")) + if !a.checkInvitePreconditions(opts, username, generatedUsername) { return 1 } - // Everything the operator typed is validated here, before anything is probed, - // asked, or disclosed: a bad value on the command line is a usage error, and a - // malformed command must never get as far as a question. Only the values that - // have to be *discovered* (a Host that must be prompted for or detected, a port - // read from sshd) are settled later, after the login check has had its say. - if *hostFlag != "" && !validate.Host(*hostFlag) { - a.errorf("%s", a.P.M("Host 不合法："+*hostFlag, "invalid host: "+*hostFlag)) - return 1 - } - if portSet && !validate.Port(*portFlag) { - a.errorf("%s", a.P.M(fmt.Sprintf("SSH 端口不合法：%d", *portFlag), fmt.Sprintf("invalid SSH port: %d", *portFlag))) - return 1 - } - if fYes && *hostFlag == "" { - a.errorf("%s", a.P.M("--yes 模式请显式传入 --host", "--yes mode requires an explicit --host")) - return 1 - } - if grantSudo == "yes" && fYes && generatedUsername { - // The confirmation names the account being granted root. With a generated - // name there is nothing to name yet: the old message interpolated this - // run's throwaway name, and the next run generated a different one, so an - // automated caller could never satisfy it. - a.errorf("%s", a.P.M( - "通过 --sudo --yes 授权必须显式指定 --user <名称> 并传入同名的 --confirm-sudo <名称>：随机生成的用户名无法事先确认", - "granting sudo via --sudo --yes requires an explicit --user NAME together with a matching --confirm-sudo NAME; a generated username cannot be confirmed in advance")) - return 1 - } - if grantSudo == "yes" && fYes && *confirmSudo != username { - a.errorf("%s", a.P.M("通过 --sudo --yes 授权需同时传入 --confirm-sudo "+username, - "granting sudo via --sudo --yes also requires --confirm-sudo "+username)) - return 1 - } - if err := user.CheckPidfd(); err != nil { - a.errorf("%s: %v", a.P.M("当前内核或进程沙箱不支持安全的进程撤销，拒绝创建无法可靠清理的账号", - "the kernel or process sandbox does not support safe process revocation; refusing to create an account that cannot be reliably removed"), err) - return 1 - } - - // Settle how the invitee will log in FIRST. planLogin only reads (`sshd -T`) - // and decides — it changes nothing — and it is the one question that can make - // every other one moot: on a host whose sshd refuses this account outright, the - // operator hears so immediately, having been asked nothing. - // - // That ordering is load-bearing, not cosmetic. Resolving the Host can involve - // asking an external echo service for this server's public IP, and this tool's - // own rule is that a root-run tool must not phone home unasked. Doing it for an - // invite that is about to be refused would be exactly that: a pointless - // disclosure of the server's address, plus two questions (sudo, auto-delete) - // whose answers were never going to be used. - // - // It also lets the confirmation below state the login method and its price in - // one summary, rather than springing a second question after the operator has - // already typed YES. - plan, ok := a.planLogin(username, fPasswordLogin, triState(fFixSSHD, fNoFixSSHD), fYes) + // Check login before resolving Host: discovery may contact an external IP + // service. Rejected invites must not cause that disclosure or needless prompts. + // The final summary includes any SSH change before the operator confirms. + plan, ok := a.planLogin(username, opts.passwordLogin, opts.fixSSHD, opts.yes) if !ok { return 1 } // A Host that was not given has to be detected or asked for; whatever comes back // is untrusted input and is validated like any other. - host := *hostFlag + host := opts.host if host == "" { host = a.detectOrPromptHost() if !validate.Host(host) { @@ -240,8 +61,8 @@ func (a *App) invite(args []string) int { } } - port := *portFlag - if !portSet { + port := opts.port + if !opts.portSet { var err error port, err = a.detectSSHPort() if err != nil { @@ -261,63 +82,35 @@ func (a *App) invite(args []string) int { // "Sudo: yes" and can be declined, and `--no-sudo` makes a plain account. A // non-interactive run (--yes) is left as a plain account unless --sudo is // passed explicitly, which keeps the --confirm-sudo gate and scripts intact. - if fYes { + if opts.yes { grantSudo = "no" } else { grantSudo = "yes" } } - if autoRev == "ask" { - if fYes { - autoRev = "yes" - } else { - answer, answered := a.promptYesNo(a.P.M("是否到期后自动删除该用户？[Y/n]: ", "Auto-delete this user on expiry? [Y/n]: "), true) - if !answered { - return 1 - } - if answer { - autoRev = "yes" - } else { - autoRev = "no" - } - } - } - - // The lifetime only means something when the account will auto-delete; without - // it the account is permanent (no expiry, no deletion), so there is nothing to - // ask. A menu-driven operator never touches --hours, so offer it here when - // auto-delete is on and it was not set on the command line. The TTY gate - // matters: promptHours re-asks on invalid input, and an unbounded non-TTY stdin - // stream (e.g. `yes n | lta invite`) never reaches EOF, so without it the root - // tool would spin forever on the pipe. - if autoRev == "yes" && !hoursSet && !fYes && a.StdinIsTTY() { - hours = a.promptHours(hours) - } - // --hours with --no-auto-revoke asks for a lifetime the permanent account will - // not have; say so rather than silently ignoring the flag. - if autoRev == "no" && hoursSet { - a.warnf("%s", a.P.M("未选择自动删除，账号将永久有效，--hours 被忽略。", - "auto-delete is off, so the account is permanent and --hours is ignored.")) + hours, wantAuto, ok := a.resolveInviteLifetime(opts.autoRevoke, opts.hours, opts.hoursSet, opts.yes) + if !ok { + return 1 } // Work out what would have to be installed BEFORE the summary, so the summary // can name it and the YES can be its consent. This only decides — the install // itself is a host change and waits until after the confirmation. - depPkgs, ok := a.planDeps(grantSudo == "yes", plan.password, fInstallDeps, fNoInstallDeps, fYes) + depPkgs, ok := a.planDeps(grantSudo == "yes", plan.password, opts.installDeps, opts.noInstallDeps, opts.yes) if !ok { return 1 } - if !fYes { + if !opts.yes { // A permanent account (auto-delete off) has no lifetime, so the summary shows // its expiry as "permanent" instead of an hours figure that would not apply. lifetime := fmt.Sprintf(a.P.M("有效期=%d小时", "expires-in=%dh"), hours) - if autoRev != "yes" { + if !wantAuto { lifetime = a.P.M("永久", "permanent") } a.printf("\n%s\n user=%s host=%s port=%d %s sudo=%s auto-delete=%s\n login=%s\n", a.P.M("即将创建一次性临时账号：", "About to create a one-time temporary account:"), - username, host, port, lifetime, a.choiceDisplay(grantSudo), a.choiceDisplay(autoRev), a.loginSummary(plan, username)) + username, host, port, lifetime, a.choiceDisplay(grantSudo), a.choiceDisplay(ynStr(wantAuto)), a.loginSummary(plan, username)) if len(depPkgs) > 0 { a.printf(" %s%s", a.P.M("确认后将安装依赖：", "dependencies to install on confirm: "), strings.Join(depPkgs, " ")) } @@ -336,7 +129,7 @@ func (a *App) invite(args []string) int { } return a.withAccountExclusiveLock(username, func() int { return a.withLifecycleLock(func() int { - return a.runInviteWithIdentityPolicy(username, host, port, hours, grantSudo == "yes", autoRev == "yes", plan, generatedUsername) + return a.runInviteWithIdentityPolicy(username, host, port, hours, grantSudo == "yes", wantAuto, plan, generatedUsername) }) }) } @@ -352,24 +145,56 @@ func (a *App) choiceDisplay(value string) string { } } -// promptHours asks for the account lifetime, offering current as the default a -// blank line accepts. It loops until the input is valid or blank. Callers must -// gate it on a.StdinIsTTY(): a closed stdin reads empty and settles on the -// default, but an unbounded non-TTY stream of invalid lines never blanks and -// would spin here forever, so it is only ever reached on a real terminal. -func (a *App) promptHours(current int) int { - msg := fmt.Sprintf(a.P.M("有效期（小时，1-%d）[%d]: ", "Lifetime in hours (1-%d) [%d]: "), - config.MaxExpireHours, current) +// resolveInviteLifetime combines the default terminal flow into one question. +// Explicit flags and piped input keep their existing auto-delete choice rules. +func (a *App) resolveInviteLifetime(autoRev string, hours int, hoursSet, yes bool) (int, bool, bool) { + wantAuto := autoRev != "no" + if !yes && !hoursSet && wantAuto && a.StdinIsTTY() { + return a.promptLifetime(hours, autoRev == "ask") + } + if autoRev == "ask" && !yes { + var ok bool + wantAuto, ok = a.promptYesNo(a.P.M("是否到期后自动删除该用户？[Y/n]: ", "Auto-delete this user on expiry? [Y/n]: "), true) + if !ok { + return hours, false, false + } + } + if !wantAuto && hoursSet { + a.warnf("%s", a.P.M("未选择自动删除，账号将永久有效，--hours 被忽略。", + "auto-delete is off, so the account is permanent and --hours is ignored.")) + } + return hours, wantAuto, true +} + +// promptLifetime accepts permanence only through an explicit keyword. A typo +// never disables expiry; EOF cancels and invalid piped input cannot loop. +func (a *App) promptLifetime(current int, allowPermanent bool) (int, bool, bool) { + msg := fmt.Sprintf(a.P.M("有效期（小时，1-%d）[%d]: ", "Lifetime in hours (1-%d) [%d]: "), config.MaxExpireHours, current) + if allowPermanent { + msg = fmt.Sprintf(a.P.M("有效期（小时，1-%d；永久请输入 never）[%d]: ", + "Lifetime in hours (1-%d; never for a permanent account) [%d]: "), config.MaxExpireHours, current) + } for { - ans := a.prompt(msg) - if ans == "" { - return current + fmt.Fprint(a.Err, msg) + answer, ok := a.readLine() + if !ok { + a.warnf("%s", a.P.M("输入已结束，已取消", "input ended; cancelled")) + return current, false, false } - if n, err := strconv.Atoi(ans); err == nil && validate.Hours(n) { - return n + if answer == "" { + return current, true, true + } + if allowPermanent && strings.EqualFold(answer, "never") { + return current, false, true + } + if hours, err := strconv.Atoi(answer); err == nil && validate.Hours(hours) { + return hours, true, true } a.warnf("%s", a.P.M(fmt.Sprintf("请输入 1-%d 之间的整数", config.MaxExpireHours), fmt.Sprintf("enter an integer between 1 and %d", config.MaxExpireHours))) + if !a.StdinIsTTY() { + return current, false, false + } } } diff --git a/internal/cli/invite_lifetime_test.go b/internal/cli/invite_lifetime_test.go new file mode 100644 index 0000000..aa07709 --- /dev/null +++ b/internal/cli/invite_lifetime_test.go @@ -0,0 +1,127 @@ +package cli + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/xxvcc/linux-temp-admin/internal/config" + "github.com/xxvcc/linux-temp-admin/internal/i18n" + "github.com/xxvcc/linux-temp-admin/internal/sysinfo" +) + +func TestPromptLifetimeKeepsExpiryUnlessPermanenceIsExplicit(t *testing.T) { + for _, tc := range []struct { + name, input string + allowPermanent, tty bool + hours int + automatic, ok bool + }{ + {"default", "\n", true, true, 24, true, true}, + {"custom", "48\n", true, true, 48, true, true}, + {"maximum", fmt.Sprint(config.MaxExpireHours) + "\n", true, true, config.MaxExpireHours, true, true}, + {"permanent", "never\n", true, true, 24, false, true}, + {"permanent keyword case", "NEVER\n", true, true, 24, false, true}, + {"typos do not disable expiry", "n\nno\n0\n-1\n99999999\n24\n", true, true, 24, true, true}, + {"explicit automatic rejects permanent", "never\n12\n", false, true, 12, true, true}, + {"EOF cancels", "", true, true, 24, false, false}, + {"EOF after typo cancels", "nevre\n", true, true, 24, false, false}, + {"invalid pipe stops", "wrong\nnever\n", true, false, 24, false, false}, + } { + t.Run(tc.name, func(t *testing.T) { + a, _, _ := newTestApp(t, tc.input) + a.StdinIsTTY = func() bool { return tc.tty } + hours, automatic, ok := a.promptLifetime(24, tc.allowPermanent) + if hours != tc.hours || automatic != tc.automatic || ok != tc.ok { + t.Fatalf("got (%d, %v, %v), want (%d, %v, %v)", hours, automatic, ok, tc.hours, tc.automatic, tc.ok) + } + }) + } +} + +// Stop at the final confirmation to exercise the real invite planning and +// prompt order without account mutations. Explicit flags and piped input retain +// their existing behavior; the default terminal flow has one lifetime prompt. +func TestInviteLifetimePlanningAndConfirmation(t *testing.T) { + for _, tc := range []struct { + name, input string + args []string + tty bool + lifetime string + automatic string + lifetimePrompts, autoPrompts int + }{ + {"terminal default", "\nNO\n", nil, true, "expires-in=24h", "yes", 1, 0}, + {"terminal custom", "48\nNO\n", nil, true, "expires-in=48h", "yes", 1, 0}, + {"terminal permanent", "never\nNO\n", nil, true, "permanent", "no", 1, 0}, + {"explicit auto", "12\nNO\n", []string{"--auto-revoke"}, true, "expires-in=12h", "yes", 1, 0}, + {"explicit permanent", "NO\n", []string{"--no-auto-revoke"}, true, "permanent", "no", 0, 0}, + {"explicit auto and hours", "NO\n", []string{"--auto-revoke", "--hours", "12"}, true, "expires-in=12h", "yes", 0, 0}, + {"hours with explicit permanent", "NO\n", []string{"--no-auto-revoke", "--hours", "12"}, true, "permanent", "no", 0, 0}, + {"hours with choice", "n\nNO\n", []string{"--hours", "12"}, true, "permanent", "no", 0, 1}, + {"pipe automatic", "y\nNO\n", nil, false, "expires-in=24h", "yes", 0, 1}, + {"pipe permanent", "n\nNO\n", nil, false, "permanent", "no", 0, 1}, + } { + t.Run(tc.name, func(t *testing.T) { + a, out, errb := invitePlanningApp(t, tc.input, tc.tty) + args := append([]string{"--user", "lta-plan", "--host", "192.0.2.1", "--port", "22", "--no-sudo"}, tc.args...) + if rc := a.invite(args); rc != 0 { + t.Fatalf("cancelled invite rc=%d: %s", rc, errb.String()) + } + summary := out.String() + for _, want := range []string{tc.lifetime, "auto-delete=" + tc.automatic, "sudo=no"} { + if !strings.Contains(summary, want) { + t.Fatalf("summary missing %q: %s", want, summary) + } + } + for prompt, want := range map[string]int{"Lifetime in hours": tc.lifetimePrompts, "Auto-delete this user": tc.autoPrompts, "Type YES to confirm": 1} { + if got := strings.Count(errb.String(), prompt); got != want { + t.Fatalf("%q count=%d, want %d: %s", prompt, got, want, errb.String()) + } + } + if _, err := os.Stat(a.Registry.File); !os.IsNotExist(err) { + t.Fatalf("cancelled invite changed registry: %v", err) + } + }) + } +} + +func TestInviteLifetimeEOFDoesNotReachConfirmation(t *testing.T) { + a, out, errb := invitePlanningApp(t, "", true) + if rc := a.invite([]string{"--user", "lta-plan", "--host", "192.0.2.1", "--port", "22", "--no-sudo"}); rc != 1 { + t.Fatalf("EOF rc=%d, want 1", rc) + } + if strings.Contains(out.String(), "About to create") || strings.Contains(errb.String(), "Type YES") { + t.Fatalf("EOF was mistaken for a lifetime selection: %s %s", out.String(), errb.String()) + } +} + +func TestPromptLifetimeChineseExplainsPermanentKeyword(t *testing.T) { + a, _, errb := newTestApp(t, "never\n") + a.P = i18n.Printer{Lang: i18n.ZH} + _, automatic, ok := a.promptLifetime(24, true) + if !ok || automatic || !strings.Contains(errb.String(), "永久请输入 never") { + t.Fatalf("permanent option was not clear in Chinese: %s", errb.String()) + } +} + +func invitePlanningApp(t *testing.T, input string, tty bool) (*App, *strings.Builder, *strings.Builder) { + t.Helper() + a, _, _ := newTestApp(t, input) + out, errb := new(strings.Builder), new(strings.Builder) + a.Out, a.Err = out, errb + a.StdinIsTTY = func() bool { return tty } + a.SSHDConfig = func(string) (*sysinfo.SSHDConfig, error) { + return sysinfo.ParseSSHD("pubkeyauthentication yes\nauthorizedkeysfile .ssh/authorized_keys\n"), nil + } + binDir := t.TempDir() + for _, name := range []string{"id", "useradd", "usermod", "chage", "userdel", "groupdel"} { + if err := os.WriteFile(filepath.Join(binDir, name), []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { + t.Fatal(err) + } + } + t.Setenv("PATH", binDir) + return a, out, errb +} diff --git a/internal/cli/invite_options.go b/internal/cli/invite_options.go new file mode 100644 index 0000000..1307f28 --- /dev/null +++ b/internal/cli/invite_options.go @@ -0,0 +1,216 @@ +package cli + +import ( + "flag" + "fmt" + + "github.com/xxvcc/linux-temp-admin/internal/config" + "github.com/xxvcc/linux-temp-admin/internal/user" + "github.com/xxvcc/linux-temp-admin/internal/validate" +) + +// inviteOptions holds parsed command-line choices, before host discovery and +// interactive defaults. Tri-state values are resolved at the planning stage. +type inviteOptions struct { + prefix, username, host string + port, hours int + portSet, hoursSet bool + sudo, autoRevoke, fixSSHD string + confirmSudo string + + yes, allowNonTTY bool + installDeps, noInstallDeps bool + passwordLogin bool +} + +func (a *App) parseInviteOptions(args []string) (inviteOptions, bool) { + fs := flag.NewFlagSet("invite", flag.ContinueOnError) + fs.SetOutput(a.Err) + prefix := fs.String("prefix", config.DefaultPrefix, "") + userFlag := fs.String("user", "", "") + hostFlag := fs.String("host", "", "") + portFlag := fs.Int("port", 0, "") + hoursFlag := fs.Int("hours", config.DefaultExpireHours, "") + confirmSudo := fs.String("confirm-sudo", "", "") + var fSudo, fNoSudo, fNopasswd, fAuto, fNoAuto, fYes, fAllowNonTTY, fInstallDeps, fNoInstallDeps bool + var fFixSSHD, fNoFixSSHD, fPasswordLogin bool + fs.BoolVar(&fSudo, "sudo", false, "") + fs.BoolVar(&fNoSudo, "no-sudo", false, "") + fs.BoolVar(&fNopasswd, "nopasswd-sudo", false, "") // deprecated alias of --sudo + fs.BoolVar(&fAuto, "auto-revoke", false, "") + fs.BoolVar(&fNoAuto, "no-auto-revoke", false, "") + fs.BoolVar(&fYes, "yes", false, "") + fs.BoolVar(&fYes, "y", false, "") + fs.BoolVar(&fAllowNonTTY, "allow-non-tty-private-key-output", false, "") + fs.BoolVar(&fInstallDeps, "install-deps", false, "") + fs.BoolVar(&fNoInstallDeps, "no-install-deps", false, "") + fs.BoolVar(&fFixSSHD, "fix-sshd", false, "") + fs.BoolVar(&fNoFixSSHD, "no-fix-sshd", false, "") + fs.BoolVar(&fPasswordLogin, "password-login", false, "") + if !a.parseFlags(fs, args) { + return inviteOptions{}, false + } + if (fSudo || fNopasswd) && fNoSudo { + a.errorf("%s", a.P.M("--sudo/--nopasswd-sudo 与 --no-sudo 互斥", + "--sudo/--nopasswd-sudo and --no-sudo are mutually exclusive")) + return inviteOptions{}, false + } + if fAuto && fNoAuto { + a.errorf("%s", a.P.M("--auto-revoke 与 --no-auto-revoke 互斥", + "--auto-revoke and --no-auto-revoke are mutually exclusive")) + return inviteOptions{}, false + } + if fInstallDeps && fNoInstallDeps { + a.errorf("%s", a.P.M("--install-deps 与 --no-install-deps 互斥", + "--install-deps and --no-install-deps are mutually exclusive")) + return inviteOptions{}, false + } + if fFixSSHD && fNoFixSSHD { + a.errorf("%s", a.P.M("--fix-sshd 与 --no-fix-sshd 互斥", "--fix-sshd and --no-fix-sshd are mutually exclusive")) + return inviteOptions{}, false + } + if fNopasswd { + fSudo = true + } + if fPasswordLogin && fFixSSHD { + a.errorf("%s", a.P.M("--password-login 与 --fix-sshd 互斥：密码登录的前提正是不改动 sshd", + "--password-login and --fix-sshd are mutually exclusive: password login exists precisely to leave sshd alone")) + return inviteOptions{}, false + } + portSet, hoursSet := false, false + fs.Visit(func(fl *flag.Flag) { + switch fl.Name { + case "port": + portSet = true + case "hours": + hoursSet = true + } + }) + + hours := *hoursFlag + if !validate.Hours(hours) { + a.errorf("%s", a.P.M(fmt.Sprintf("--hours 必须在 1..%d 之间", config.MaxExpireHours), + fmt.Sprintf("--hours must be between 1 and %d", config.MaxExpireHours))) + return inviteOptions{}, false + } + if !validate.Prefix(*prefix) { + a.errorf("%s", a.P.M("用户名前缀不合法："+*prefix, "invalid username prefix: "+*prefix)) + return inviteOptions{}, false + } + return inviteOptions{ + prefix: *prefix, username: *userFlag, host: *hostFlag, + port: *portFlag, hours: hours, portSet: portSet, hoursSet: hoursSet, + sudo: triState(fSudo, fNoSudo), autoRevoke: triState(fAuto, fNoAuto), fixSSHD: triState(fFixSSHD, fNoFixSSHD), + confirmSudo: *confirmSudo, yes: fYes, allowNonTTY: fAllowNonTTY, + installDeps: fInstallDeps, noInstallDeps: fNoInstallDeps, passwordLogin: fPasswordLogin, + }, true +} + +func (a *App) resolveInviteUsername(username, prefix string) (string, bool, bool) { + generatedUsername := username == "" + if username == "" { + // Only the generation path uses the prefix. A prefix in the reserved + // "systemd-" namespace would generate usernames the revoke path refuses to + // delete (user.IsReservedName), so reject it here before generating. An + // explicit --user does not use the prefix and is validated on its own below. + if user.IsReservedName(prefix + "-") { + a.errorf("%s", a.P.M("用户名前缀落入受保护命名空间（如 systemd-），会创建无法撤销的账号："+prefix, + "username prefix is in a reserved namespace (e.g. systemd-) and would create an unrevocable account: "+prefix)) + return "", false, false + } + // Fill the username's remaining Linux-compatible length with entropy. Even + // the longest accepted prefix retains the historical 40-bit minimum, while + // the default prefix receives 104 bits. + suffixBytes := (31 - len(prefix)) / 2 + for attempt := 0; attempt < 20; attempt++ { + h, err := a.RandHex(suffixBytes) + if err != nil { + a.errorf("rand: %v", err) + return "", false, false + } + cand := prefix + "-" + h + // Dependency planning happens later and may need to install `id`. Use the + // local database while choosing a candidate, then perform the authoritative + // local+NSS check inside the lifecycle lock immediately before creation. + exists, lookupErr := user.Exists(cand) + if lookupErr != nil { + a.errorf("%s: %v", a.P.M("读取账号数据库失败", "reading account database failed"), lookupErr) + return "", false, false + } + if !exists { + username = cand + break + } + } + if username == "" { + a.errorf("%s", a.P.M("随机用户名多次冲突，请指定 --user", "random username collided repeatedly; specify --user")) + return "", false, false + } + } + if !validate.Username(username) { + a.errorf("%s", a.P.M("用户名不合法："+username, "invalid username: "+username)) + return "", false, false + } + // Refuse a reserved/system name (root, daemon, systemd-*, ...): the revoke path + // protects these, so creating one would leave an account the tool can never + // delete — manually or via the auto-revoke timer. This is the authoritative + // gate; it also covers an explicit --user that bypasses the prefix path above. + if user.IsReservedName(username) { + a.errorf("%s", a.P.M("用户名落入受保护/系统命名空间，拒绝创建（撤销将无法删除）："+username, + "username is a reserved/system name and cannot be created (revoke would refuse to delete it): "+username)) + return "", false, false + } + + return username, generatedUsername, true +} + +// checkInvitePreconditions runs before prompts, host discovery or mutations. +func (a *App) checkInvitePreconditions(opts inviteOptions, username string, generatedUsername bool) bool { + // Refuse a non-TTY stdout up front — before any prompt or host probe — so a + // piped run fails immediately rather than after the operator answers. + if !a.StdoutIsTTY() && !opts.allowNonTTY { + a.errorf("%s", a.P.M("stdout 非 TTY，拒绝输出一次性私钥/密码（可加 --allow-non-tty-private-key-output）", + "stdout is not a TTY; refusing to print the one-time private key or password (add --allow-non-tty-private-key-output)")) + return false + } + + // Everything the operator typed is validated here, before anything is probed, + // asked, or disclosed: a bad value on the command line is a usage error, and a + // malformed command must never get as far as a question. Only the values that + // have to be *discovered* (a Host that must be prompted for or detected, a port + // read from sshd) are settled later, after the login check has had its say. + if opts.host != "" && !validate.Host(opts.host) { + a.errorf("%s", a.P.M("Host 不合法："+opts.host, "invalid host: "+opts.host)) + return false + } + if opts.portSet && !validate.Port(opts.port) { + a.errorf("%s", a.P.M(fmt.Sprintf("SSH 端口不合法：%d", opts.port), fmt.Sprintf("invalid SSH port: %d", opts.port))) + return false + } + if opts.yes && opts.host == "" { + a.errorf("%s", a.P.M("--yes 模式请显式传入 --host", "--yes mode requires an explicit --host")) + return false + } + if opts.sudo == "yes" && opts.yes && generatedUsername { + // The confirmation names the account being granted root. With a generated + // name there is nothing to name yet: the old message interpolated this + // run's throwaway name, and the next run generated a different one, so an + // automated caller could never satisfy it. + a.errorf("%s", a.P.M( + "通过 --sudo --yes 授权必须显式指定 --user <名称> 并传入同名的 --confirm-sudo <名称>：随机生成的用户名无法事先确认", + "granting sudo via --sudo --yes requires an explicit --user NAME together with a matching --confirm-sudo NAME; a generated username cannot be confirmed in advance")) + return false + } + if opts.sudo == "yes" && opts.yes && opts.confirmSudo != username { + a.errorf("%s", a.P.M("通过 --sudo --yes 授权需同时传入 --confirm-sudo "+username, + "granting sudo via --sudo --yes also requires --confirm-sudo "+username)) + return false + } + if err := user.CheckPidfd(); err != nil { + a.errorf("%s: %v", a.P.M("当前内核或进程沙箱不支持安全的进程撤销，拒绝创建无法可靠清理的账号", + "the kernel or process sandbox does not support safe process revocation; refusing to create an account that cannot be reliably removed"), err) + return false + } + + return true +} diff --git a/internal/cli/invite_reuse_root_test.go b/internal/cli/invite_reuse_root_test.go index a885a86..d575f3a 100644 --- a/internal/cli/invite_reuse_root_test.go +++ b/internal/cli/invite_reuse_root_test.go @@ -1030,9 +1030,9 @@ func TestInviteInteractiveDefaultsSudoOn(t *testing.T) { a, _, _, _ := inviteApp(t) out := a.Out.(*bytes.Buffer) a.StdinIsTTY = func() bool { return true } - // Interactive answers: sudo is NOT asked now; auto-delete [Y/n] -> n (so no - // hours prompt either); then the confirmation YES. - a.In = strings.NewReader("n\nYES\n") + // The default terminal flow asks for lifetime once. Explicitly select a + // permanent account, then confirm the summary that includes sudo. + a.In = strings.NewReader("never\nYES\n") const name = "xxvcc-defsudo1" integrationtest.RequireUserAbsent(t, name, true) t.Cleanup(func() { integrationtest.CleanupUser(t, name, true) }) diff --git a/internal/cli/narrow_terminal_test.go b/internal/cli/narrow_terminal_test.go index 029c4c9..a28bfcf 100644 --- a/internal/cli/narrow_terminal_test.go +++ b/internal/cli/narrow_terminal_test.go @@ -4,9 +4,11 @@ import ( "strings" "testing" + "github.com/xxvcc/linux-temp-admin/internal/config" "github.com/xxvcc/linux-temp-admin/internal/i18n" "github.com/xxvcc/linux-temp-admin/internal/registry" "github.com/xxvcc/linux-temp-admin/internal/table" + "github.com/xxvcc/linux-temp-admin/internal/user" ) func TestTakeDisplayWidthConsumesInvalidByteWithoutPanic(t *testing.T) { @@ -74,3 +76,49 @@ func TestMenuLabelsFitFortyColumns(t *testing.T) { } } } + +func TestUsersViewUsesOneObservationPerAccountAndRefreshes(t *testing.T) { + for _, tc := range []struct { + name string + width int + }{{"table", 200}, {"cards", 40}} { + t.Run(tc.name, func(t *testing.T) { + a, _, _ := newTestApp(t, "") + a.TerminalWidth = func() int { return tc.width } + const generation = "0123456789abcdef0123456789abcdef" + recs := []registry.Record{ + {User: "lta-view-a", UID: 1001, Generation: generation, IdentityBound: true, Port: 22}, + {User: "lta-view-b", UID: 1002, Generation: generation, IdentityBound: true, Port: 22}, + } + calls := map[string]int{} + a.LookupUser = func(name string) (user.Passwd, bool, error) { + calls[name]++ + if calls[name] > 1 { + return user.Passwd{}, false, nil + } + for _, rec := range recs { + if rec.User == name { + return user.Passwd{Name: name, UID: rec.UID, GID: rec.UID, + GECOS: ",,,," + config.ManagedGenerationGECOSWitnessPrefix + generation, + Home: "/home/" + name, Shell: "/bin/sh"}, true, nil + } + } + t.Fatalf("unexpected lookup: %s", name) + return user.Passwd{}, false, nil + } + first := a.usersView(recs, true) + if strings.Count(first, "active") != len(recs) || strings.Contains(first, "missing") { + t.Fatalf("first display did not preserve its account observations:\n%s", first) + } + for _, rec := range recs { + if calls[rec.User] != 1 { + t.Fatalf("first render looked up %s %d times", rec.User, calls[rec.User]) + } + } + second := a.usersView(recs, true) + if strings.Count(second, "missing") != len(recs) || strings.Contains(second, "active") { + t.Fatalf("refresh reused stale account observations:\n%s", second) + } + }) + } +} diff --git a/internal/cli/revoke.go b/internal/cli/revoke.go index 3cc138f..16155c0 100644 --- a/internal/cli/revoke.go +++ b/internal/cli/revoke.go @@ -924,22 +924,7 @@ func (a *App) teardownLocalAccountWith( if err := stillMatches(username, expected); err != nil { return revokeDeleteAccount, err } - // Persist recovery authority before controlled mail/Home cleanup begins. The - // account can disappear out of band at any later syscall boundary; without this - // witness, a failed post-disappearance mail fsync could be mistaken on retry for - // an ordinary stale row and discarded without completing the narrow cleanup. - if persistDeletion == nil { - return revokeDeleteAccount, fmt.Errorf("deletion recovery persistence is not configured") - } - if err := persistDeletion(); err != nil { - return revokeDeleteAccount, fmt.Errorf("persist deletion-started recovery state: %w", err) - } - if err := deleteExpected(username, expected, func() error { - return a.finalScheduledAccountCheckWith(username, expected, stillMatches) - }); err != nil { - return revokeDeleteAccount, err - } - return revokeAccountRemoved, nil + return a.persistAndDeleteAccount(username, expected, persistDeletion, stillMatches, deleteExpected) } func (a *App) teardownQuarantinedAccount(username string, expected user.Passwd, persistDeletion func() error, removePrivateGroup bool) (revokeAccountStage, error) { @@ -952,18 +937,35 @@ func (a *App) teardownQuarantinedAccount(username string, expected user.Passwd, if err := a.quiesceScheduledAccountImmediateForRevoke(username, expected); err != nil { return revokeQuiesceAccount, err } + deleteExpected := a.Users.DeleteExpected + if removePrivateGroup { + deleteExpected = a.Users.DeleteExpectedSequential + } + return a.persistAndDeleteAccount(username, expected, persistDeletion, a.revokeAccountStillMatches, deleteExpected) +} + +// persistAndDeleteAccount is the shared final step after the caller's chosen +// quiescence checks. The callbacks preserve the identity policy used by revoke +// or an in-flight invite rollback, including the last check inside deletion. +func (a *App) persistAndDeleteAccount( + username string, + expected user.Passwd, + persistDeletion func() error, + stillMatches func(string, user.Passwd) error, + deleteExpected func(string, user.Passwd, func() error) error, +) (revokeAccountStage, error) { + // Persist recovery authority before controlled mail/Home cleanup begins. The + // account can disappear out of band at any later syscall boundary; without this + // witness, a failed post-disappearance mail fsync could be mistaken on retry for + // an ordinary stale row and discarded without completing the narrow cleanup. if persistDeletion == nil { return revokeDeleteAccount, fmt.Errorf("deletion recovery persistence is not configured") } if err := persistDeletion(); err != nil { return revokeDeleteAccount, fmt.Errorf("persist deletion-started recovery state: %w", err) } - deleteExpected := a.Users.DeleteExpected - if removePrivateGroup { - deleteExpected = a.Users.DeleteExpectedSequential - } if err := deleteExpected(username, expected, func() error { - return a.finalScheduledAccountCheck(username, expected) + return a.finalScheduledAccountCheckWith(username, expected, stillMatches) }); err != nil { return revokeDeleteAccount, err } @@ -1243,14 +1245,10 @@ func (a *App) quiesceScheduledAccountImmediateWith(username string, expected use return errors.Join(errs...) } -// finalScheduledAccountCheck runs after controlled Home/mail cleanup and just +// finalScheduledAccountCheckWith runs after controlled Home/mail cleanup and just // before userdel. The earlier drain already waited out daemon-side cached work; // this last pass terminates processes first, then closes jobs raced in during // filesystem cleanup without imposing a second polling-cycle delay. -func (a *App) finalScheduledAccountCheck(username string, expected user.Passwd) error { - return a.finalScheduledAccountCheckWith(username, expected, a.revokeAccountStillMatches) -} - func (a *App) finalScheduledAccountCheckWith(username string, expected user.Passwd, stillMatches func(string, user.Passwd) error) error { if err := stillMatches(username, expected); err != nil { return err diff --git a/internal/cli/revoke_test.go b/internal/cli/revoke_test.go index 0748512..ffb3dba 100644 --- a/internal/cli/revoke_test.go +++ b/internal/cli/revoke_test.go @@ -2111,7 +2111,7 @@ func TestFinalScheduledAccountCheckClearsWorkQueuedBeforeTerminationCompletes(t }, } - if err := a.finalScheduledAccountCheck(pw.Name, pw); err != nil { + if err := a.finalScheduledAccountCheckWith(pw.Name, pw, a.revokeAccountStillMatches); err != nil { t.Fatal(err) } if queued { diff --git a/internal/cli/selfcmd.go b/internal/cli/selfcmd.go index 020f1c1..2364c69 100644 --- a/internal/cli/selfcmd.go +++ b/internal/cli/selfcmd.go @@ -383,29 +383,22 @@ func detachedSignatureURL(binaryURL string) (string, error) { } func (a *App) upgradePreparedLocked(candidate *selfmanage.UpgradeCandidate, force bool) commandResult { - previous := "" - if v, err := a.Selfmanage.InstalledVersion(); err == nil { - previous = v - } else if !errors.Is(err, selfmanage.ErrNotInstalled) { - previous = "unknown" - } - newVer, err := a.Selfmanage.ApplyUpgrade(candidate, force) + upgrade, err := a.Selfmanage.ApplyUpgrade(candidate, force) if err != nil { - var durability *fsutil.DurabilityError - if newVer != "" && errors.As(err, &durability) { + if upgrade.Replaced { a.errorf("%s: %v", a.P.M("命令已替换，但无法确认升级已持久化", "the command was replaced, but the upgrade's durability is unknown"), err) - a.audit("upgrade", "", "fail", versionTransition(previous, newVer)+" visible but durability unknown: "+err.Error(), nil) + a.audit("upgrade", "", "fail", versionTransition(upgrade.PreviousVersion, upgrade.Version)+" visible but durability unknown: "+err.Error(), nil) } else { a.errorf("%s: %v", a.P.M("升级失败", "upgrade failed"), err) a.audit("upgrade", "", "fail", "upgrade failed before replacement: "+err.Error(), nil) } - return statusResult(1) + return commandResult{status: 1, applied: upgrade.Replaced} } - if newVer == "" { + if !upgrade.Replaced { return a.reportNoopUpgrade(a.Selfmanage.InstalledVersion) } - a.audit("upgrade", "", "ok", versionTransition(previous, newVer), nil) - a.success(a.P.M("已升级到 "+newVer, "upgraded to "+newVer)) + a.audit("upgrade", "", "ok", versionTransition(upgrade.PreviousVersion, upgrade.Version), nil) + a.success(a.P.M("已升级到 "+upgrade.Version, "upgraded to "+upgrade.Version)) return commandResult{applied: true} } diff --git a/internal/cli/selfcmd_result_test.go b/internal/cli/selfcmd_result_test.go new file mode 100644 index 0000000..50b77c4 --- /dev/null +++ b/internal/cli/selfcmd_result_test.go @@ -0,0 +1,169 @@ +//go:build integration + +package cli + +import ( + "bytes" + "crypto/ed25519" + "crypto/rand" + "encoding/json" + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "runtime" + "strings" + "syscall" + "testing" + + "github.com/xxvcc/linux-temp-admin/internal/audit" + "github.com/xxvcc/linux-temp-admin/internal/config" + "github.com/xxvcc/linux-temp-admin/internal/fsutil" + "github.com/xxvcc/linux-temp-admin/internal/lifecycle" + "github.com/xxvcc/linux-temp-admin/internal/selfmanage" +) + +func TestUpgradeMenuRetiresProcessAfterVisibleReplacement(t *testing.T) { + for _, tc := range []struct { + name string + installed string + writeFailure string + wantStatus int + wantReplaced bool + wantWrites int + wantMainPrompts int + }{ + {name: "successful replacement", installed: "2.0.0", wantReplaced: true, wantWrites: 1, wantMainPrompts: 1}, + {name: "replacement with durability failure", installed: "2.0.0", writeFailure: "after", wantStatus: 1, wantReplaced: true, wantWrites: 1, wantMainPrompts: 1}, + {name: "failure before replacement", installed: "2.0.0", writeFailure: "before", wantStatus: 1, wantWrites: 1, wantMainPrompts: 2}, + {name: "already current", installed: "2.0.1", wantMainPrompts: 2}, + } { + t.Run(tc.name, func(t *testing.T) { + dir := rootOwnedDir(t) + upgradeChoice, exitChoice := 0, 0 + for i, item := range menuItems { + if item.en == "Upgrade" { + upgradeChoice = i + 1 + } + if item.run == nil { + exitChoice = i + 1 + } + } + if upgradeChoice == 0 || exitChoice == 0 { + t.Fatal("real menu is missing upgrade or exit") + } + a, out, errb := newTestApp(t, fmt.Sprintf("%d\nYES\n%d\n", upgradeChoice, exitChoice)) + a.InstallPath = filepath.Join(dir, "linux-temp-admin") + a.Lifecycle = lifecycle.New(filepath.Join(dir, "lifecycle.lock")) + auditDir := filepath.Join(dir, "audit") + auditPath := filepath.Join(auditDir, "audit.log") + a.Audit = &audit.Logger{Dir: auditDir, File: auditPath, Now: a.Now, Actor: func() (string, int) { return "test", 0 }} + pub, private, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + t.Fatal(err) + } + binary := func(version string) []byte { + return []byte("#!/bin/sh\n# LTA_RELEASE_VERSION_V1{" + version + "}\n[ \"$1\" = version ] && echo " + version + "\n") + } + old, candidate := binary(tc.installed), binary("2.0.1") + signature := ed25519.Sign(private, candidate) + asset := config.BinaryAssetPrefix + runtime.GOARCH + base := config.ReleaseMirrorBaseURL + "/v2.0.1" + payloads := map[string][]byte{ + config.ReleaseMirrorManifestURL: []byte(fmt.Sprintf(`{"version":"2.0.1","tag":"v2.0.1","base_url":%q,"published_at":"2026-07-27T05:00:00Z"}`, base) + "\n"), + base + "/SHA256SUMS": releaseSetSums(asset, candidate, signature), + base + "/" + asset: candidate, + base + "/" + asset + ".sig": signature, + } + m := &selfmanage.Manager{ + InstallPath: a.InstallPath, RequireHostMachine: cliAllowAnyMachine, + PublicKey: pub, MaxBytes: config.MaxUpgradeBytes, + Client: &http.Client{Transport: cliRoundTripFunc(func(req *http.Request) (*http.Response, error) { + body, ok := payloads[req.URL.String()] + if !ok { + return nil, fmt.Errorf("unexpected fixture request: %s", req.URL) + } + return &http.Response{StatusCode: http.StatusOK, Header: make(http.Header), Body: io.NopCloser(bytes.NewReader(body)), Request: req}, nil + })}, + } + a.Selfmanage = m + if _, err := m.Install(old, false); err != nil { + t.Fatal(err) + } + before, err := os.Stat(a.InstallPath) + if err != nil { + t.Fatal(err) + } + writeCalls := 0 + m.WriteRootFile = func(path string, content []byte, mode os.FileMode) error { + writeCalls++ + if tc.writeFailure == "before" { + return syscall.EIO + } + if err := fsutil.WriteRootFile(path, content, mode); err != nil { + return err + } + if tc.writeFailure == "after" { + return &fsutil.DurabilityError{Operation: "rename", Err: syscall.EIO} + } + return nil + } + + if status := a.menu(); status != tc.wantStatus { + t.Fatalf("menu status=%d, want %d; %s", status, tc.wantStatus, errb) + } + if writeCalls != tc.wantWrites { + t.Fatalf("upgrade writer called %d times, want %d; %s", writeCalls, tc.wantWrites, errb) + } + mainPrompt := fmt.Sprintf("select [1-%d] (Enter shows the menu): ", len(menuItems)) + if prompts := strings.Count(errb.String(), mainPrompt); prompts != tc.wantMainPrompts { + t.Fatalf("menu prompted %d times, want %d after visible replacement=%t; %s", prompts, tc.wantMainPrompts, tc.wantReplaced, errb) + } + wantBytes := old + if tc.wantReplaced { + wantBytes = candidate + } + actual, readErr := os.ReadFile(a.InstallPath) + if readErr != nil || !bytes.Equal(actual, wantBytes) { + t.Fatalf("visible command bytes match=%t, err=%v", bytes.Equal(actual, wantBytes), readErr) + } + after, statErr := os.Stat(a.InstallPath) + if statErr != nil || os.SameFile(before, after) == tc.wantReplaced { + t.Fatalf("actual inode replacement does not match expected=%t: %v", tc.wantReplaced, statErr) + } + if tc.writeFailure != "" { + wantDiagnostic := "upgrade failed" + wantAudit := "upgrade failed before replacement: " + if tc.writeFailure == "after" { + wantDiagnostic = "upgrade's durability is unknown" + wantAudit = "2.0.0 -> 2.0.1 visible but durability unknown: " + } + if !strings.Contains(errb.String(), wantDiagnostic) || !strings.Contains(errb.String(), syscall.EIO.Error()) || strings.Contains(out.String(), "upgraded to") { + t.Fatalf("injected write failure diagnostic is wrong: stdout=%q stderr=%q", out, errb) + } + events, readErr := os.ReadFile(auditPath) + if readErr != nil { + t.Fatal(readErr) + } + matched := false + for _, line := range bytes.Split(bytes.TrimSpace(events), []byte("\n")) { + var event struct { + Action string `json:"action"` + Result string `json:"result"` + Detail string `json:"detail"` + } + if err := json.Unmarshal(line, &event); err != nil { + t.Fatal(err) + } + if event.Action == "upgrade" && event.Result == "fail" && strings.Contains(event.Detail, wantAudit) && strings.Contains(event.Detail, syscall.EIO.Error()) { + matched = true + } + } + if !matched { + t.Fatalf("injected write failure missing from audit: %s", events) + } + } + }) + } +} diff --git a/internal/schedule/valid.go b/internal/schedule/valid.go index efb45a5..aaae265 100644 --- a/internal/schedule/valid.go +++ b/internal/schedule/valid.go @@ -90,14 +90,7 @@ func (s *Scheduler) ValidSchedule(user string, uid int, generation, recordedUnit if recordedUnit != unit || strings.ContainsAny(unit, "/ ") { return false, nil } - service, valid, err := readScheduleFile(filepath.Join(s.SystemdDir, unit+".service")) - if err != nil || !valid { - return false, err - } - if string(service) != s.serviceContent(user, uid, generation) { - return false, nil - } - timer, valid, err := readScheduleFile(filepath.Join(s.SystemdDir, unit+".timer")) + timer, valid, err := s.readBoundTimer(user, uid, generation, unit) if err != nil || !valid { return false, err } @@ -149,14 +142,7 @@ func (s *Scheduler) ValidQuarantine(user string, uid int, generation, recordedUn if recordedUnit != unit || strings.ContainsAny(unit, "/ ") { return false, nil } - service, valid, err := readScheduleFile(filepath.Join(q.SystemdDir, unit+".service")) - if err != nil || !valid { - return false, err - } - if string(service) != q.serviceContent(user, uid, generation) { - return false, nil - } - timer, valid, err := readScheduleFile(filepath.Join(q.SystemdDir, unit+".timer")) + timer, valid, err := q.readBoundTimer(user, uid, generation, unit) if err != nil || !valid { return false, err } @@ -174,6 +160,19 @@ func (s *Scheduler) ValidQuarantine(user string, uid int, generation, recordedUn return q.systemdTimerExecutable(unit + ".timer") } +// readBoundTimer reads a timer only after its service names the exact account +// identity. Callers keep their own calendar and legacy-format acceptance rules. +func (s *Scheduler) readBoundTimer(user string, uid int, generation, unit string) ([]byte, bool, error) { + service, valid, err := readScheduleFile(filepath.Join(s.SystemdDir, unit+".service")) + if err != nil || !valid { + return nil, false, err + } + if string(service) != s.serviceContent(user, uid, generation) { + return nil, false, nil + } + return readScheduleFile(filepath.Join(s.SystemdDir, unit+".timer")) +} + func (s *Scheduler) systemdTimerExecutable(timer string) (bool, error) { if s.Sys == nil { return false, fmt.Errorf("query systemd timer %s: no system backend", timer) diff --git a/internal/selfmanage/selfmanage.go b/internal/selfmanage/selfmanage.go index 6617621..3011af8 100644 --- a/internal/selfmanage/selfmanage.go +++ b/internal/selfmanage/selfmanage.go @@ -37,7 +37,7 @@ import ( type Manager struct { InstallPath string // PublicKey is the legacy single-key injection point. PublicKeys is the - // rotation-capable keyring; Upgrade accepts a signature made by either. New + // rotation-capable keyring; candidate verification accepts signatures from either. New // populates both so existing callers that inspect PublicKey keep working. PublicKey ed25519.PublicKey PublicKeys []ed25519.PublicKey @@ -171,8 +171,7 @@ func New(installPath string, maxBytes int64) *Manager { // Install atomically writes srcBytes to InstallPath as a root-owned 0755 binary. // It reports whether it actually wrote: a byte-identical target is left alone and -// returns (false, nil), mirroring Upgrade's ("", nil) for "nothing to do". If the -// target differs and force is false, it refuses. +// returns (false, nil). If the target differs and force is false, it refuses. func (m *Manager) Install(srcBytes []byte, force bool) (installed bool, err error) { if err := ensureInstallDir(filepath.Dir(m.InstallPath)); err != nil { return false, err @@ -632,52 +631,72 @@ func (m *Manager) prepareVerifiedCandidate(bin, sig []byte, expectedVersion stri }, nil } +// UpgradeResult records the installed state observed at commit time and whether +// the command was visibly replaced. Replaced remains true if directory syncing +// fails after replacement, so an interactive caller can retire its old process. +type UpgradeResult struct { + // PreviousVersion is empty when no command was installed, or "unknown" when + // an existing command could not be identified. + PreviousVersion string + // Version is the resulting version after replacement or a successful no-op. + // It remains empty when an error prevented a visible replacement. + Version string + Replaced bool +} + // ApplyUpgrade re-reads the installed command at commit time, applies the // downgrade policy to that current state, and atomically installs candidate. -// It returns ("", nil) if the installed command is already the same version or -// newer. If replacement is visible but not known durable, the version is returned -// alongside the durability error so the CLI can report the partial outcome. -func (m *Manager) ApplyUpgrade(candidate *UpgradeCandidate, force bool) (string, error) { +// A same/newer installed command produces a successful result with Replaced +// false. A visible but not known durable replacement returns both its result +// and the durability error; failure alone does not mean nothing changed. +func (m *Manager) ApplyUpgrade(candidate *UpgradeCandidate, force bool) (UpgradeResult, error) { + var result UpgradeResult if candidate == nil || len(candidate.bin) == 0 || (candidate.signedVersion != "" && !validate.ReleaseVersion(candidate.signedVersion)) || (candidate.expected != "" && !validate.ReleaseVersion(candidate.expected)) { - return "", fmt.Errorf("invalid prepared upgrade candidate") + return result, fmt.Errorf("invalid prepared upgrade candidate") } - installedVersion := "" if current, err := m.InstalledVersion(); err == nil { - installedVersion = current - } else if !errors.Is(err, ErrNotInstalled) && !force { - return "", fmt.Errorf("read installed version: %w", err) + result.PreviousVersion = current + } else if !errors.Is(err, ErrNotInstalled) { + result.PreviousVersion = "unknown" + if !force { + return result, fmt.Errorf("read installed version: %w", err) + } } if !force { if candidate.signedVersion == "" { - return "", fmt.Errorf("signed candidate has no static release-version witness; use --force only after independently confirming the historical binary") + return result, fmt.Errorf("signed candidate has no static release-version witness; use --force only after independently confirming the historical binary") } - if installedVersion != "" && !version.Greater(candidate.signedVersion, installedVersion) { - return "", nil // already up to date or newer; candidate was not executed + if result.PreviousVersion != "" && !version.Greater(candidate.signedVersion, result.PreviousVersion) { + result.Version = result.PreviousVersion + return result, nil // same/newer install; candidate was not executed } } probedVersion, err := m.probeVersion(candidate.bin) if err != nil { - return "", fmt.Errorf("read downloaded version: %w", err) + return result, fmt.Errorf("read downloaded version: %w", err) } if candidate.signedVersion != "" && probedVersion != candidate.signedVersion { - return "", fmt.Errorf("candidate version %q does not match signed release-version witness %q", probedVersion, candidate.signedVersion) + return result, fmt.Errorf("candidate version %q does not match signed release-version witness %q", probedVersion, candidate.signedVersion) } if candidate.expected != "" && probedVersion != candidate.expected { - return "", fmt.Errorf("signed candidate version %q does not match selected release %q", probedVersion, candidate.expected) + return result, fmt.Errorf("signed candidate version %q does not match selected release %q", probedVersion, candidate.expected) + } + result.Replaced, err = m.Install(candidate.bin, true) + if result.Replaced { + result.Version = probedVersion } - installed, err := m.Install(candidate.bin, true) if err != nil { - if installed { - return probedVersion, fmt.Errorf("installed command was replaced but durability is unknown: %w", err) + if result.Replaced { + return result, fmt.Errorf("installed command was replaced but durability is unknown: %w", err) } - return "", err - } - if !installed { - return "", nil + return result, err } - return probedVersion, nil + // Install also returns false for identical, already-safe bytes. This is a + // known no-op, not an unverified or failed installation. + result.Version = probedVersion + return result, nil } var releaseVersionWitnessPrefix = []byte{ @@ -713,17 +732,6 @@ func releaseVersionWitness(bin []byte) (string, error) { return versionValue, nil } -// Upgrade is the one-shot API retained for callers that already provide their -// own serialization. CLI code uses PrepareUpgrade and ApplyUpgrade separately so -// network retries never hold the global lifecycle lock. -func (m *Manager) Upgrade(binaryURL, sigURL string, force bool) (string, error) { - candidate, err := m.PrepareUpgrade(binaryURL, sigURL) - if err != nil { - return "", err - } - return m.ApplyUpgrade(candidate, force) -} - func (m *Manager) verificationKeys() []ed25519.PublicKey { keys := make([]ed25519.PublicKey, 0, len(m.PublicKeys)+1) seen := make(map[string]struct{}) diff --git a/internal/selfmanage/selfmanage_root_test.go b/internal/selfmanage/selfmanage_root_test.go index 98a98bf..114dbca 100644 --- a/internal/selfmanage/selfmanage_root_test.go +++ b/internal/selfmanage/selfmanage_root_test.go @@ -319,12 +319,12 @@ func TestUpgradeVerifiesSignatureAndInstalls(t *testing.T) { srv := signedServer(t, bin, sig) m := &Manager{RequireHostMachine: allowAnyMachine, InstallPath: filepath.Join(dir, "linux-temp-admin"), PublicKey: pub, Client: srv.Client(), MaxBytes: 1 << 20} - got, err := m.Upgrade(srv.URL+"/bin", srv.URL+"/sig", false) + got, err := prepareAndApplyUpgrade(m, srv.URL+"/bin", srv.URL+"/sig", false) if err != nil { t.Fatalf("Upgrade: %v", err) } - if got != "2.0.1" { - t.Errorf("new version = %q, want 2.0.1", got) + if got.Version != "2.0.1" || !got.Replaced { + t.Errorf("new result = %+v, want visible 2.0.1 replacement", got) } if b, _ := os.ReadFile(m.InstallPath); string(b) != string(bin) { t.Error("installed binary does not match the downloaded one") @@ -351,8 +351,8 @@ func TestPreparedUpgradeRechecksInstalledVersionAtCommit(t *testing.T) { if wrote, err := m.Install(newBinary("3.0.0"), true); err != nil || !wrote { t.Fatalf("concurrent newer install: wrote=%v err=%v", wrote, err) } - if got, err := m.ApplyUpgrade(candidate, false); err != nil || got != "" { - t.Fatalf("ApplyUpgrade over newer install: version=%q err=%v", got, err) + if got, err := m.ApplyUpgrade(candidate, false); err != nil || got != (UpgradeResult{PreviousVersion: "3.0.0", Version: "3.0.0"}) { + t.Fatalf("ApplyUpgrade over newer install: result=%+v err=%v", got, err) } if current, err := m.InstalledVersion(); err != nil || current != "3.0.0" { t.Fatalf("prepared candidate downgraded newer install: version=%q err=%v", current, err) @@ -376,8 +376,8 @@ func TestPreparedUpgradeRejectsDowngradeBeforeExecutingCandidate(t *testing.T) { if _, err := os.Lstat(evidence); !os.IsNotExist(err) { t.Fatalf("candidate executed during preparation: %v", err) } - if got, err := m.ApplyUpgrade(candidate, false); err != nil || got != "" { - t.Fatalf("ApplyUpgrade downgrade: version=%q err=%v", got, err) + if got, err := m.ApplyUpgrade(candidate, false); err != nil || got.Replaced { + t.Fatalf("ApplyUpgrade downgrade: result=%+v err=%v", got, err) } if _, err := os.Lstat(evidence); !os.IsNotExist(err) { t.Fatalf("candidate executed before downgrade refusal: %v", err) @@ -396,15 +396,15 @@ func TestHistoricalSignedUpgradeRequiresForceBeforeProbe(t *testing.T) { if err != nil || candidate.Version() != "" { t.Fatalf("prepare historical candidate: version=%q err=%v", candidate.Version(), err) } - if got, err := m.ApplyUpgrade(candidate, false); got != "" || err == nil || + if got, err := m.ApplyUpgrade(candidate, false); got.Replaced || err == nil || !strings.Contains(err.Error(), "no static release-version witness") { - t.Fatalf("non-forced historical candidate: version=%q err=%v", got, err) + t.Fatalf("non-forced historical candidate: result=%+v err=%v", got, err) } if _, err := os.Lstat(evidence); !os.IsNotExist(err) { t.Fatalf("historical candidate executed without --force: %v", err) } - if got, err := m.ApplyUpgrade(candidate, true); err != nil || got != "2.0.0" { - t.Fatalf("forced historical candidate: version=%q err=%v", got, err) + if got, err := m.ApplyUpgrade(candidate, true); err != nil || got.Version != "2.0.0" || !got.Replaced { + t.Fatalf("forced historical candidate: result=%+v err=%v", got, err) } if content, err := os.ReadFile(evidence); err != nil || string(content) != "executed" { t.Fatalf("forced candidate probe evidence: content=%q err=%v", content, err) @@ -426,9 +426,9 @@ func TestForcedHistoricalCandidateStillMustMatchSelectedRelease(t *testing.T) { t.Fatalf("prepare historical candidate: %v", err) } - if got, err := m.ApplyUpgrade(candidate, true); got != "" || err == nil || + if got, err := m.ApplyUpgrade(candidate, true); got.Replaced || err == nil || !strings.Contains(err.Error(), "does not match selected release") { - t.Fatalf("forced mismatched historical candidate: version=%q err=%v", got, err) + t.Fatalf("forced mismatched historical candidate: result=%+v err=%v", got, err) } if content, err := os.ReadFile(evidence); err != nil || string(content) != "executed" { t.Fatalf("bounded probe evidence: content=%q err=%v", content, err) @@ -447,7 +447,7 @@ func TestUpgradeRejectsBadSignature(t *testing.T) { srv := signedServer(t, bin, badSig) m := &Manager{RequireHostMachine: allowAnyMachine, InstallPath: filepath.Join(dir, "linux-temp-admin"), PublicKey: pub, Client: srv.Client(), MaxBytes: 1 << 20} - if _, err := m.Upgrade(srv.URL+"/bin", srv.URL+"/sig", false); err == nil { + if _, err := prepareAndApplyUpgrade(m, srv.URL+"/bin", srv.URL+"/sig", false); err == nil { t.Fatal("Upgrade must reject a bad signature") } if _, err := os.Lstat(m.InstallPath); !os.IsNotExist(err) { @@ -466,12 +466,12 @@ func TestUpgradeSkipsWhenNotNewer(t *testing.T) { if installed, err := m.Install(bin, false); err != nil || !installed { t.Fatalf("seed installed version: installed=%v err=%v", installed, err) } - got, err := m.Upgrade(srv.URL+"/bin", srv.URL+"/sig", false) + got, err := prepareAndApplyUpgrade(m, srv.URL+"/bin", srv.URL+"/sig", false) if err != nil { t.Fatal(err) } - if got != "" { - t.Errorf("expected no upgrade (same version), got %q", got) + if got.Replaced { + t.Errorf("expected no upgrade (same version), got %+v", got) } if b, err := os.ReadFile(m.InstallPath); err != nil || string(b) != string(bin) { t.Errorf("same installed version changed: content=%q err=%v", b, err) @@ -498,15 +498,18 @@ func TestUpgradeUsesInstalledCommandAsVersionBaseline(t *testing.T) { t.Fatalf("seed installed command: wrote=%v err=%v", wrote, err) } - got, err := m.Upgrade(srv.URL+"/bin", srv.URL+"/sig", false) + got, err := prepareAndApplyUpgrade(m, srv.URL+"/bin", srv.URL+"/sig", false) if err != nil { t.Fatal(err) } - if tc.want == tc.candidate && got != tc.candidate { - t.Fatalf("Upgrade version=%q, want %q", got, tc.candidate) + if tc.want == tc.candidate && (got.Version != tc.candidate || !got.Replaced) { + t.Fatalf("Upgrade result=%+v, want %q", got, tc.candidate) } - if tc.want == tc.installed && got != "" { - t.Fatalf("newer installed command was downgraded without --force: Upgrade=%q", got) + if tc.want == tc.installed && got.Replaced { + t.Fatalf("newer installed command was downgraded without --force: Upgrade=%+v", got) + } + if got.PreviousVersion != tc.installed || got.Version != tc.want { + t.Fatalf("Upgrade state = %+v, want previous=%q current=%q", got, tc.installed, tc.want) } current, err := m.InstalledVersion() if err != nil || current != tc.want { @@ -516,23 +519,56 @@ func TestUpgradeUsesInstalledCommandAsVersionBaseline(t *testing.T) { } } -func TestUpgradeReturnsCandidateWithDurabilityFailure(t *testing.T) { - dir := rootDir(t) - pub, priv, _ := ed25519.GenerateKey(rand.Reader) - bin := newBinary("2.0.1") - srv := signedServer(t, bin, ed25519.Sign(priv, bin)) - m := &Manager{RequireHostMachine: allowAnyMachine, InstallPath: filepath.Join(dir, "linux-temp-admin"), PublicKey: pub, Client: srv.Client(), MaxBytes: 1 << 20} - m.WriteRootFile = func(path string, content []byte, mode os.FileMode) error { - if err := fsutil.WriteRootFile(path, content, mode); err != nil { - return err - } - return &fsutil.DurabilityError{Operation: "rename", Err: syscall.EIO} - } +func TestUpgradeReportsWhetherFailedWriteReplacedCommand(t *testing.T) { + for _, committed := range []bool{false, true} { + t.Run(fmt.Sprintf("committed=%t", committed), func(t *testing.T) { + dir := rootDir(t) + pub, priv, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + t.Fatal(err) + } + old := newBinary("2.0.0") + bin := newBinary("2.0.1") + srv := signedServer(t, bin, ed25519.Sign(priv, bin)) + m := &Manager{RequireHostMachine: allowAnyMachine, InstallPath: filepath.Join(dir, "linux-temp-admin"), PublicKey: pub, Client: srv.Client(), MaxBytes: 1 << 20} + if _, err := m.Install(old, false); err != nil { + t.Fatal(err) + } + before, err := os.Stat(m.InstallPath) + if err != nil { + t.Fatal(err) + } + m.WriteRootFile = func(path string, content []byte, mode os.FileMode) error { + if !committed { + return syscall.EIO + } + if err := fsutil.WriteRootFile(path, content, mode); err != nil { + return err + } + return &fsutil.DurabilityError{Operation: "rename", Err: syscall.EIO} + } - got, err := m.Upgrade(srv.URL+"/bin", srv.URL+"/sig", false) - var durability *fsutil.DurabilityError - if got != "2.0.1" || !errors.As(err, &durability) { - t.Fatalf("Upgrade = version=%q err=%v, want candidate plus DurabilityError", got, err) + got, err := prepareAndApplyUpgrade(m, srv.URL+"/bin", srv.URL+"/sig", false) + var durability *fsutil.DurabilityError + if !errors.Is(err, syscall.EIO) || errors.As(err, &durability) != committed || got.Replaced != committed || got.PreviousVersion != "2.0.0" { + t.Fatalf("Upgrade = %+v, %v; want committed=%t and previous=2.0.0", got, err, committed) + } + wantBytes, wantVersion := old, "" + if committed { + wantBytes, wantVersion = bin, "2.0.1" + } + if got.Version != wantVersion { + t.Fatalf("result version=%q, want %q", got.Version, wantVersion) + } + actual, readErr := os.ReadFile(m.InstallPath) + if readErr != nil || !bytes.Equal(actual, wantBytes) { + t.Fatalf("visible command bytes match=%t, err=%v", bytes.Equal(actual, wantBytes), readErr) + } + after, statErr := os.Stat(m.InstallPath) + if statErr != nil || os.SameFile(before, after) == committed { + t.Fatalf("inode replacement does not match committed=%t: %v", committed, statErr) + } + }) } } @@ -548,8 +584,8 @@ func TestUpgradeAcceptsAnyKeyInKeyring(t *testing.T) { Client: srv.Client(), MaxBytes: 1 << 20, } - if got, err := m.Upgrade(srv.URL+"/bin", srv.URL+"/sig", false); err != nil || got != "2.0.1" { - t.Fatalf("Upgrade with secondary key: version=%q err=%v", got, err) + if got, err := prepareAndApplyUpgrade(m, srv.URL+"/bin", srv.URL+"/sig", false); err != nil || got.Version != "2.0.1" || !got.Replaced { + t.Fatalf("Upgrade with secondary key: result=%+v err=%v", got, err) } } @@ -614,7 +650,7 @@ func TestCandidateWhoseProbeDisagreesWithItsSignedWitnessIsRefused(t *testing.T) got, err := m.ApplyUpgrade(candidate, false) if err == nil { - t.Fatalf("ApplyUpgrade = %q, nil; a candidate that reports a different version than it is signed for must be refused", got) + t.Fatalf("ApplyUpgrade = %+v, nil; a candidate that reports a different version than it is signed for must be refused", got) } if !strings.Contains(err.Error(), "does not match signed release-version witness") { t.Fatalf("error = %v, want the signed-witness mismatch refusal", err) diff --git a/internal/selfmanage/selfmanage_test.go b/internal/selfmanage/selfmanage_test.go index 6275bcf..9b52e43 100644 --- a/internal/selfmanage/selfmanage_test.go +++ b/internal/selfmanage/selfmanage_test.go @@ -39,6 +39,17 @@ type failingResponseBody struct{ err error } func (b failingResponseBody) Read([]byte) (int, error) { return 0, b.err } func (failingResponseBody) Close() error { return nil } +// prepareAndApplyUpgrade exercises both production phases without retaining a +// redundant one-shot production API. CLI callers hold their lifecycle lock only +// for ApplyUpgrade, after preparation has finished. +func prepareAndApplyUpgrade(m *Manager, binaryURL, sigURL string, force bool) (UpgradeResult, error) { + candidate, err := m.PrepareUpgrade(binaryURL, sigURL) + if err != nil { + return UpgradeResult{}, err + } + return m.ApplyUpgrade(candidate, force) +} + func TestEmbeddedPublicKeyConfigured(t *testing.T) { keys := embeddedPublicKeys() if len(keys) == 0 { @@ -276,7 +287,7 @@ func TestInstallerDropsImportedShellFunctions(t *testing.T) { func TestUpgradeRefusedWithoutKey(t *testing.T) { m := &Manager{PublicKey: nil} - if _, err := m.Upgrade("https://x/bin", "https://x/sig", false); err == nil { + if _, err := prepareAndApplyUpgrade(m, "https://x/bin", "https://x/sig", false); err == nil { t.Error("Upgrade must refuse when no signing key is configured") } }