This is an automated email from the ASF dual-hosted git repository. hsluoyz pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/casbin-gateway.git
commit a634eb372c13b89f027f5e9e00471edb46d24606 Author: Yang Luo <[email protected]> AuthorDate: Fri Sep 4 18:07:27 2026 +0800 fix(agents): write a machine-wide agent's config into the right home --- agenthome/home.go | 24 ++++++++++++++--- util/port_unix.go => agenthome/home_unix.go | 16 ++++------- agenthome/home_windows.go | 42 +++++++++++++++++++++++++++++ agenthook/hook.go | 11 +++----- util/port.go | 8 +++--- util/port_unix.go | 3 +-- util/port_windows.go | 27 ++++++------------- 7 files changed, 82 insertions(+), 49 deletions(-) diff --git a/agenthome/home.go b/agenthome/home.go index 6d76543..a091d70 100644 --- a/agenthome/home.go +++ b/agenthome/home.go @@ -31,6 +31,11 @@ import ( // happens to run Gateway. func Resolve(owner string) (string, error) { owner = strings.TrimSpace(owner) + // A machine-wide installation is stamped with the local system account + // rather than a person, and its configuration belongs to whoever is using it. + if machineWideOwner(owner) { + owner = "" + } if owner == "" { home, err := os.UserHomeDir() if err != nil { @@ -40,10 +45,7 @@ func Resolve(owner string) (string, error) { } if account, err := lookupAccount(owner); err == nil { - if account.HomeDir == "" { - return "", fmt.Errorf("account %q has no home directory", owner) - } - return account.HomeDir, nil + return usableHome(owner, account.HomeDir) } // user.Lookup is unreliable for domain-qualified names on Windows and for @@ -59,6 +61,20 @@ func Resolve(owner string) (string, error) { return "", fmt.Errorf("cannot resolve the home directory of %q; run Gateway as that user to patch this installation", owner) } +// usableHome expands a home the operating system reported and refuses one that +// is still relative, which would write an agent's configuration under Gateway's +// working directory and report success. +func usableHome(owner string, home string) (string, error) { + home = strings.TrimSpace(expandHome(home)) + if home == "" { + return "", fmt.Errorf("account %q has no home directory", owner) + } + if !filepath.IsAbs(home) { + return "", fmt.Errorf("account %q reports %q as its home directory, which is not an absolute path", owner, home) + } + return filepath.Clean(home), nil +} + // lookupAccount resolves an owner name, retrying with the bare account name so // that DOMAIN\user and user@domain forms also resolve. func lookupAccount(owner string) (*user.User, error) { diff --git a/util/port_unix.go b/agenthome/home_unix.go similarity index 61% copy from util/port_unix.go copy to agenthome/home_unix.go index 2919141..2fb52be 100644 --- a/util/port_unix.go +++ b/agenthome/home_unix.go @@ -14,16 +14,10 @@ //go:build !windows -package util +package agenthome -import ( - "strconv" - "strings" -) +// Both are Windows-only: no scan here stamps an installation with the machine +// rather than a person, and home directories come back as literal paths. +func machineWideOwner(string) bool { return false } -// findProcessName resolves a pid to an executable name, returning "" when the -// platform command is missing or says nothing useful. "ps" is asked about the -// one pid, so it stays fast however many processes the host runs. -func findProcessName(pid int) string { - return strings.TrimSpace(runLookup("ps", "-p", strconv.Itoa(pid), "-o", "comm=")) -} +func expandHome(home string) string { return home } diff --git a/agenthome/home_windows.go b/agenthome/home_windows.go new file mode 100644 index 0000000..cba03dc --- /dev/null +++ b/agenthome/home_windows.go @@ -0,0 +1,42 @@ +// Copyright 2026 The casbin Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package agenthome + +import ( + "strings" + + "golang.org/x/sys/windows/registry" +) + +// systemAccount is what the Windows scan stamps on an installation found in a +// machine-wide location; it is a marker, not an account that runs an agent. +const systemAccount = "SYSTEM" + +// machineWideOwner reports whether an owner names the local system account. +func machineWideOwner(owner string) bool { + return strings.EqualFold(accountName(owner), systemAccount) +} + +// expandHome resolves the "%SystemRoot%" that os/user hands back unexpanded for +// Windows built-in accounts. +func expandHome(home string) string { + if !strings.Contains(home, "%") { + return home + } + if expanded, err := registry.ExpandString(home); err == nil && expanded != "" { + return expanded + } + return home +} diff --git a/agenthook/hook.go b/agenthook/hook.go index b88e054..6ebafb0 100644 --- a/agenthook/hook.go +++ b/agenthook/hook.go @@ -37,12 +37,9 @@ const ( maxHookInput = 8 * 1024 * 1024 reportTimeout = 5 * time.Second - // hookLifetime is how long the whole hook process may live. The report has - // its own timeout, but reading the event does not: an agent that starts a - // hook and then never writes the event, or never closes the pipe, leaves - // this process blocked in Decode forever. One copy of Gateway's executable - // per such event adds up - they are tens of megabytes each and only the - // machine's owner ever notices - so the process puts a deadline on itself. + // hookLifetime bounds the whole process: the report has a timeout, reading + // the event does not, and an event that never arrives leaks a copy of this + // executable. hookLifetime = 30 * time.Second ) @@ -53,8 +50,6 @@ func ServeIfInvoked() { if len(os.Args) < 2 || os.Args[1] != Subcommand { return } - // Exit 0 either way: a hook that fails is a record not kept, never an agent - // action held up or refused. timer := time.AfterFunc(hookLifetime, func() { os.Exit(0) }) defer timer.Stop() _ = Run(os.Args[2:], os.Stdin) diff --git a/util/port.go b/util/port.go index 2ca5349..f13786f 100644 --- a/util/port.go +++ b/util/port.go @@ -28,11 +28,9 @@ import ( ) const ( - // portLookupTimeout bounds the external "who owns this port" commands so a - // slow lsof or netstat can never hold up startup. It is generous because a - // timeout here is not a slow answer but a wrong one: an unnamed holder is - // taken for a foreign program, and a previous Gateway that cannot be named - // is never stopped. + // portLookupTimeout bounds the external "who owns this port" commands. A + // timeout here is not a slow answer but a wrong one: a holder that cannot + // be named is taken for a foreign program and never stopped. portLookupTimeout = 10 * time.Second // procNameMaxLen is how much of another process's name Linux reports, from // TASK_COMM_LEN. Anything longer comes back cut to this. diff --git a/util/port_unix.go b/util/port_unix.go index 2919141..ef9c119 100644 --- a/util/port_unix.go +++ b/util/port_unix.go @@ -22,8 +22,7 @@ import ( ) // findProcessName resolves a pid to an executable name, returning "" when the -// platform command is missing or says nothing useful. "ps" is asked about the -// one pid, so it stays fast however many processes the host runs. +// platform command is missing or says nothing useful. func findProcessName(pid int) string { return strings.TrimSpace(runLookup("ps", "-p", strconv.Itoa(pid), "-o", "comm=")) } diff --git a/util/port_windows.go b/util/port_windows.go index 3082260..5306ecd 100644 --- a/util/port_windows.go +++ b/util/port_windows.go @@ -22,44 +22,33 @@ import ( "golang.org/x/sys/windows" ) -// imagePathMaxLen is the buffer a full executable path is read into. Windows -// paths reach 32767 characters, but a process image path that long cannot be -// launched in the first place, so one MAX_PATH-sized try and one large one are -// all this ever needs. const imagePathMaxLen = 32768 // findProcessName resolves a pid to an executable name such as "nginx.exe". -// -// The obvious way to do this on Windows is "tasklist", but tasklist enumerates -// and formats every process on the machine: on a host with a thousand of them -// it takes over two seconds, and a lookup that times out reports no name at -// all. That is not a cosmetic loss - an unnamed holder is treated as a foreign -// program, so "stop" refuses to stop Gateway's own server and a restart cannot -// reclaim its own port. Asking the kernel about the one pid we care about -// answers in microseconds and cannot be starved by an unrelated process count. +// tasklist would enumerate every process on the machine, which takes over two +// seconds on a busy host and leaves the holder unnamed, so the kernel is asked +// about this one pid instead. tasklist stays as the fallback for a process this +// one may not open. func findProcessName(pid int) string { if name := processImageName(pid); name != "" { return name } - // A process owned by another account can refuse to be opened. tasklist runs - // with the same rights, but it is worth one slow try before giving up. output := runLookup("tasklist", "/FI", fmt.Sprintf("PID eq %d", pid), "/NH", "/FO", "CSV") // A CSV row looks like: "nginx.exe","1234","Console","1","5,000 K" name := strings.TrimSpace(strings.SplitN(output, ",", 2)[0]) return strings.Trim(name, `"`) } -// processImageName reads one process's executable path straight from the -// kernel, and returns just its file name. It returns "" when the process is -// gone or this one may not query it. +// processImageName returns the file name of a pid's executable, or "" when the +// process is gone or may not be queried. func processImageName(pid int) string { if pid <= 0 { return "" } - // QUERY_LIMITED_INFORMATION is the least this can ask for and still read an - // image path, which is what lets it work across accounts. + // QUERY_LIMITED_INFORMATION is the least that still reads an image path, + // which is what lets this work across accounts. handle, err := windows.OpenProcess(windows.PROCESS_QUERY_LIMITED_INFORMATION, false, uint32(pid)) if err != nil { return "" --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
