mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-03 04:54:36 +00:00
fix(packages): use writable scratch dir for github updates
Squash merge PR #97 after resolving changelog conflict with current dev. PR CI run 26703171763 passed release-versioning, go, and web.
This commit is contained in:
1 parent
c4e0bd2be5
commit
361f2abbf5
5 files changed
+125
-17
No files matched your search
@@ -72,6 +72,10 @@ token in `config.json`.
|
||||
| `GOCLAW_PACKAGES_GITHUB_BIN_DIR` | `{runtimeDir}/bin` | Where extracted binaries land |
|
||||
| `GOCLAW_PACKAGES_GITHUB_MANIFEST` | `{bin_dir}/../github-packages.json` | Manifest path |
|
||||
|
||||
`packages.scratch_dir` in `config.json` is optional. If it is empty or cannot
|
||||
be created, updates use `{runtimeDir}/tmp` so bare-metal services do not depend
|
||||
on root-owned release directories such as `/opt/goclaw/tmp`.
|
||||
|
||||
Token scopes:
|
||||
- public-only repos: no scopes required
|
||||
- private repos: `repo`
|
||||
@@ -224,7 +228,8 @@ Check response header `X-RateLimit-Reset` (Unix epoch). Wait or set
|
||||
|
||||
#### Scratch dir leftover after crash
|
||||
|
||||
Path: `{BinDir}/../tmp/{name}-{tag}-{nanos}/`
|
||||
Path: `{runtimeDir}/tmp/{name}-{tag}-{nanos}/` unless
|
||||
`packages.scratch_dir` points to another writable directory.
|
||||
|
||||
Safe to remove any `{name}-*-*` directory under tmp after ensuring no active
|
||||
update is in flight. Phase 2 will add startup GC.
|
||||
|
||||
@@ -6,6 +6,18 @@ Significant changes, features, and fixes in reverse chronological order.
|
||||
|
||||
## 2026-05-29
|
||||
|
||||
### GitHub Releases update scratch dir fallback (issue #94)
|
||||
|
||||
- Changed GitHub Releases package updates to prefer `{runtimeDir}/tmp` for
|
||||
scratch extraction and staging, instead of deriving tmp from a release or
|
||||
binary directory.
|
||||
- If `packages.scratch_dir` is configured but cannot be created, the update
|
||||
executor now logs a warning and falls back to runtime tmp before failing.
|
||||
- Added regression tests for default scratch-dir selection and fallback from an
|
||||
unusable configured scratch path.
|
||||
|
||||
---
|
||||
|
||||
### CLI Credentials git preset null-env crash (issue #93)
|
||||
|
||||
**Fixes**
|
||||
|
||||
@@ -72,9 +72,8 @@ type Config struct {
|
||||
// empty string → default 1h.
|
||||
//
|
||||
// ScratchDir is the tmp workspace used by the update executor for download
|
||||
// + extract + staging before atomic swap. Defaults to "{BinDir}/../tmp" when
|
||||
// empty; operators MAY set explicitly to avoid symlink-resolution issues
|
||||
// (red-team H6).
|
||||
// + extract + staging before atomic swap. Empty or unusable values fall back to
|
||||
// "{runtimeDir}/tmp"; operators MAY set an explicit writable path.
|
||||
type PackagesConfig struct {
|
||||
GitHubToken string `json:"github_token,omitempty"` // Phase 2 stub
|
||||
UpdatesCheckTTL string `json:"updates_check_ttl,omitempty"` // e.g. "1h"
|
||||
|
||||
@@ -26,15 +26,14 @@ var (
|
||||
// - C3: re-verifies asset via meta SHA256 when present; refuses staged
|
||||
// URL whose host is not in allowedDownloadHosts.
|
||||
// - C4: saveManifest retries up to 3× before declaring desync.
|
||||
// - H6: explicit ScratchDir (no "../tmp" symlink hazard).
|
||||
// - scratch workspace resolves to writable runtime storage before staging.
|
||||
// - L4: file written with 0755 during extraction, not chmod post-rename.
|
||||
type GitHubUpdateExecutor struct {
|
||||
Installer *GitHubInstaller
|
||||
ScratchDir string // explicit; defaults to filepath.Join(BinDir, "..", "tmp") if empty
|
||||
ScratchDir string // optional override; falls back to {runtimeDir}/tmp if unusable
|
||||
}
|
||||
|
||||
// NewGitHubUpdateExecutor wires the executor. Call SetScratchDir to override
|
||||
// the default tmp path.
|
||||
// NewGitHubUpdateExecutor wires the executor.
|
||||
func NewGitHubUpdateExecutor(installer *GitHubInstaller) *GitHubUpdateExecutor {
|
||||
return &GitHubUpdateExecutor{Installer: installer}
|
||||
}
|
||||
@@ -42,12 +41,51 @@ func NewGitHubUpdateExecutor(installer *GitHubInstaller) *GitHubUpdateExecutor {
|
||||
// Source returns "github".
|
||||
func (e *GitHubUpdateExecutor) Source() string { return "github" }
|
||||
|
||||
// scratchDir returns the resolved scratch directory.
|
||||
func (e *GitHubUpdateExecutor) scratchDir() string {
|
||||
if e.ScratchDir != "" {
|
||||
return e.ScratchDir
|
||||
// scratchDirCandidates returns scratch parents in preference order.
|
||||
func (e *GitHubUpdateExecutor) scratchDirCandidates() []string {
|
||||
candidates := make([]string, 0, 3)
|
||||
add := func(path string) {
|
||||
path = strings.TrimSpace(path)
|
||||
if path == "" {
|
||||
return
|
||||
}
|
||||
cleaned := filepath.Clean(path)
|
||||
for _, existing := range candidates {
|
||||
if existing == cleaned {
|
||||
return
|
||||
}
|
||||
}
|
||||
candidates = append(candidates, cleaned)
|
||||
}
|
||||
return filepath.Join(filepath.Dir(e.Installer.Config.BinDir), "tmp")
|
||||
|
||||
add(e.ScratchDir)
|
||||
add(filepath.Join(packageRuntimeDir(), "tmp"))
|
||||
if e.Installer != nil && e.Installer.Config != nil && e.Installer.Config.BinDir != "" {
|
||||
add(filepath.Join(filepath.Dir(e.Installer.Config.BinDir), "tmp"))
|
||||
}
|
||||
return candidates
|
||||
}
|
||||
|
||||
func (e *GitHubUpdateExecutor) createScratchDir(name, target string) (string, error) {
|
||||
suffix := fmt.Sprintf("%s-%s-%d", name, sanitizeTag(target), time.Now().UnixNano())
|
||||
var errs []error
|
||||
for idx, base := range e.scratchDirCandidates() {
|
||||
scratch := filepath.Join(base, suffix)
|
||||
if err := os.MkdirAll(scratch, 0o755); err != nil {
|
||||
errs = append(errs, fmt.Errorf("%s: %w", scratch, err))
|
||||
continue
|
||||
}
|
||||
if idx > 0 {
|
||||
slog.Warn("github.update: scratch dir fallback selected",
|
||||
"scratch", scratch,
|
||||
"previous_error", errs[0])
|
||||
}
|
||||
return scratch, nil
|
||||
}
|
||||
if len(errs) == 0 {
|
||||
return "", errors.New("create scratch dir: no scratch directory candidates")
|
||||
}
|
||||
return "", fmt.Errorf("create scratch dir: %w", errors.Join(errs...))
|
||||
}
|
||||
|
||||
// Update applies the target version. The caller holds PackageLocker for
|
||||
@@ -130,10 +168,9 @@ func (e *GitHubUpdateExecutor) Update(ctx context.Context, name, toVersion strin
|
||||
}
|
||||
|
||||
// Prepare scratch dir — isolated per-update.
|
||||
scratch := filepath.Join(e.scratchDir(),
|
||||
fmt.Sprintf("%s-%s-%d", name, sanitizeTag(target), time.Now().UnixNano()))
|
||||
if err := os.MkdirAll(scratch, 0o755); err != nil {
|
||||
return fmt.Errorf("create scratch dir: %w", err)
|
||||
scratch, err := e.createScratchDir(name, target)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
defer os.RemoveAll(scratch)
|
||||
|
||||
|
||||
@@ -246,6 +246,61 @@ func TestGitHubUpdateExecutor_NotInstalled(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestGitHubUpdateExecutor_DefaultScratchDirUsesRuntimeDir(t *testing.T) {
|
||||
runtimeDir := t.TempDir()
|
||||
t.Setenv("RUNTIME_DIR", runtimeDir)
|
||||
|
||||
cfg := &GitHubPackagesConfig{
|
||||
BinDir: filepath.Join(string(filepath.Separator), "opt", "goclaw", "bin"),
|
||||
ManifestPath: filepath.Join(t.TempDir(), "manifest.json"),
|
||||
}
|
||||
inst := NewGitHubInstaller(NewGitHubClient(""), cfg)
|
||||
|
||||
exec := NewGitHubUpdateExecutor(inst)
|
||||
candidates := exec.scratchDirCandidates()
|
||||
if len(candidates) == 0 {
|
||||
t.Fatal("scratchDirCandidates returned no candidates")
|
||||
}
|
||||
|
||||
want := filepath.Join(runtimeDir, "tmp")
|
||||
if candidates[0] != want {
|
||||
t.Fatalf("primary scratch dir = %q, want runtime tmp %q", candidates[0], want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGitHubUpdateExecutor_CreateScratchDirFallsBackFromBadConfiguredDir(t *testing.T) {
|
||||
runtimeDir := t.TempDir()
|
||||
t.Setenv("RUNTIME_DIR", runtimeDir)
|
||||
|
||||
cfg := &GitHubPackagesConfig{
|
||||
BinDir: filepath.Join(runtimeDir, "bin"),
|
||||
ManifestPath: filepath.Join(runtimeDir, "github-packages.json"),
|
||||
}
|
||||
inst := NewGitHubInstaller(NewGitHubClient(""), cfg)
|
||||
|
||||
blockedPath := filepath.Join(t.TempDir(), "not-a-directory")
|
||||
if err := os.WriteFile(blockedPath, []byte("blocked"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
exec := NewGitHubUpdateExecutor(inst)
|
||||
exec.ScratchDir = blockedPath
|
||||
|
||||
scratch, err := exec.createScratchDir("goclaw", "v0.8.0-beta.2")
|
||||
if err != nil {
|
||||
t.Fatalf("createScratchDir should fall back from bad configured dir: %v", err)
|
||||
}
|
||||
defer os.RemoveAll(scratch)
|
||||
|
||||
wantPrefix := filepath.Join(runtimeDir, "tmp") + string(filepath.Separator)
|
||||
if !strings.HasPrefix(scratch, wantPrefix) {
|
||||
t.Fatalf("scratch dir = %q, want prefix %q", scratch, wantPrefix)
|
||||
}
|
||||
if fi, statErr := os.Stat(scratch); statErr != nil || !fi.IsDir() {
|
||||
t.Fatalf("scratch dir not created: info=%v err=%v", fi, statErr)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGitHubUpdateExecutor_MetaAssertions_NilSafe(t *testing.T) {
|
||||
// Red-team C6: nil-safe map assertions must never panic.
|
||||
cases := []map[string]any{
|
||||
|
||||
Reference in new issue
Block a user