refactor(pkg-helper): extract hardcoded GID to constant and log chown errors

Replace magic number 1000 with goclawGID constant across all chown
calls. Add slog.Warn on chown failures instead of silently ignoring
them, improving debuggability for permission issues in production.
This commit is contained in:
viettranx committed 2026-03-25 08:32:00 +07:00
1 parent 18824f841d
commit 7382aceb7e
1 file changed
+14 -5
+14 -5
View File
@@ -21,6 +21,11 @@ import (
const socketPath = "/tmp/pkg.sock"
// goclawGID is the group ID of the goclaw process.
// Persist files and sockets are chowned to root:goclaw so the
// unprivileged app process (uid 1000, gid 1000) can read them.
const goclawGID = 1000
// validPkgName allows alphanumeric, hyphens, underscores, dots, @, / (scoped npm).
// Rejects names starting with - to prevent argument injection.
var validPkgName = regexp.MustCompile(`^[a-zA-Z0-9@][a-zA-Z0-9._+\-/@]*$`)
@@ -55,7 +60,7 @@ func main() {
// Chown requires CAP_CHOWN; if missing (misconfigured container), warn but continue
// since umask already set restrictive permissions.
if os.Getuid() == 0 {
if err := os.Chown(socketPath, 0, 1000); err != nil {
if err := os.Chown(socketPath, 0, goclawGID); err != nil {
slog.Warn("pkg-helper: chown socket failed (missing CAP_CHOWN?)", "error", err)
}
}
@@ -196,7 +201,9 @@ func persistAdd(pkg string) {
fmt.Fprintln(f, pkg)
// Ensure group ownership allows the goclaw process to read the file.
if created {
os.Chown(listFile, 0, 1000) //nolint:errcheck
if err := os.Chown(listFile, 0, goclawGID); err != nil {
slog.Warn("pkg-helper: chown persist file failed", "file", listFile, "error", err)
}
}
}
@@ -227,10 +234,12 @@ func persistRemove(pkg string) {
os.Remove(tmpFile) //nolint:errcheck
return
}
// Restore group ownership so the goclaw process (uid 1000, gid 1000) can read the file.
// Restore group ownership so the goclaw process (gid 1000) can read the file.
// Without this, the renamed file inherits root:root from the temp file,
// causing ListInstalledPackages to return nil for system packages.
os.Chown(listFile, 0, 1000) //nolint:errcheck
if err := os.Chown(listFile, 0, goclawGID); err != nil {
slog.Warn("pkg-helper: chown persist file failed", "file", listFile, "error", err)
}
}
func apkListFile() string {
@@ -258,7 +267,7 @@ func ensurePersistDir() {
// Try to fix ownership to root:goclaw (gid 1000) if not already root-owned.
// CAP_CHOWN is available even when CAP_DAC_OVERRIDE is dropped.
if stat, ok := fi.Sys().(*syscall.Stat_t); ok && stat.Uid != 0 {
if err := os.Chown(dir, 0, 1000); err != nil {
if err := os.Chown(dir, 0, goclawGID); err != nil {
slog.Warn("pkg-helper: cannot fix persist dir ownership", "dir", dir, "error", err)
} else {
os.Chmod(dir, 0750) //nolint:errcheck