mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-09-05 14:22:59 +00:00
* feat(packages): add update flow for GitHub binaries (#900) Closes #900. Proactive update-check + atomic swap for GitHub-installed binaries on the Runtime & Packages page. Interfaces prepared for pip/npm/apk extension in Phase 2. - UpdateCache + UpdateRegistry + PackageLocker (ctx-aware keyed mutex) - GitHubUpdateChecker: ETag-aware, distinct /latest vs /list ETag keys, semver-correct ordering via golang.org/x/mod/semver, non-semver fallback that refuses to downgrade, pre-release + stable candidate fusion for the v1.0.0-rc.1 -> v1.0.0 transition - GitHubUpdateExecutor: two-phase .bak swap with hadBackup-aware rollback, manifest save retry (3x, 100ms/500ms/1s backoff), nil-safe meta access, explicit ScratchDir, 0755 set pre-rename - HTTP: GET /v1/packages/updates (SWR), POST /v1/packages/updates/refresh, POST /v1/packages/update, POST /v1/packages/updates/apply-all (always 200, failed[] is error source). Master-scope gated. - WS events package.update.{checked,started,succeeded,failed} forwarded to owner clients via event_filter.go - Frontend: useUpdates hook + 3 components (summary bar, update-all modal, row button), master-scope-gated disabled state - i18n: 8 backend keys + 17 frontend keys x en/vi/zh - Config: packages.github_token (reserved), updates_check_ttl, scratch_dir - 45+ new tests, race-clean, BenchmarkCheckAll10Packages ~1.1ms/op warm * docs(packages): document update flow + Phase 1 completion - packages-github.md: "Updating Installed Packages" section with UI + API contract, troubleshooting runbook (corrupt cache, rate-limit, scratch dir, mid-swap recovery) - 17-changelog.md + CHANGELOG.md: Phase 1 entry - 14-skills-runtime.md: cross-ref to update flow - journal entry capturing CRIT fixes (double-write, lock-key mismatch, rollback false-alarm) + design wins (keyed locks, red-team pre-flight) * feat(workstation): remote workstation runtime — SSH exec + security + audit Adds generic Remote Workstation Runtime enabling agents to execute commands on user-owned SSH workstations. Includes registry (DB + API + UI), SSH backend with connection pool and circuit breaker, workstation.exec + claude_remote tools, NFKC + binary-name allowlist security, and audit logging. Standard edition only. Closes #941. * fix(workstation): address 3 critical + 5 important code review findings - C1: Add json:"-" to Metadata/DefaultEnv fields; use SanitizedView() in all API responses to prevent SSH private key leakage - C2: Wire CheckEnv into PermCheckFn; LD_PRELOAD/PATH injection now blocked - C3: SSH Setenv fallback — prepend `export K=V;` when server rejects Setenv - I1: BackendCache sync.RWMutex → sync.Mutex (fix data race on lastUsed) - I2: Validate metadata shape in handleUpdate before store write - I3: Include command in exec-done event; activity sink uses actual cmd hash - I4: Wrap pool release in sync.Once (idempotent double-call safety) - I5: Verify workstation tenant ownership before adding permissions * fix(packages): bypass HTTPS+IP validation in update executor tests Test httptest servers bind to http://127.0.0.1 which fails both the HTTPS scheme check and literal-IP SSRF guard. Add testSkipDownloadValidation flag (same pattern as existing withTestDownloadHosts) to skip full URL validation in test context. * fix(workstation): address Claude review findings — tenant isolation + pool leak + dead code - Activity list: add workstation ownership check before listing (prevents cross-tenant activity enumeration via known UUID) - SSH pool: clean up p.sem + p.circuits maps in CloseWorkstation, prune, and Close to prevent unbounded map growth - RPC handlers: return ErrInvalidRequest on JSON unmarshal failure instead of silently using zero-value params - Remove unused containsControlChars function in normalize.go - HTTP tests: add 10s context timeout to prevent CI package timeout * fix(workstation): DefaultEnv JSON parse, backend cache leak, perm ownership check - DefaultEnv: replace KEY=VALUE text parse with json.Unmarshal (stored as JSON by HTTP handler, was silently ignored) - BackendCache: close losing backend on concurrent cache miss to prevent pruneLoop goroutine leak - Backend interface: add Close() error method; SSHBackend delegates to pool.Close() - handlePermList: add wsStore.GetByID ownership check (prevents cross-tenant UUID enumeration returning empty array vs 404) - scanRows: log scan errors instead of silently skipping * fix(workstation): wire activity sink shutdown + remove misleading comment - WireActivitySink: capture cleanup func, register in gateway shutdown (was discarded → retention goroutine leaked + buffered rows lost) - Add Stop() to WorkstationActivityStore interface (PG+SQLite already had it) - wireWorkstationTools returns cleanup func; gateway.go defers it - Remove misleading "re-validate env" comment in allowlist.go Check() * ci: bump unit test timeout from 90s to 120s hooks/handlers package (goja script tests) consumes ~85s on cold CI runners, leaving insufficient headroom for HTTP retry tests with 1s backoff. 120s provides adequate breathing room without masking real deadlocks. * fix: compile errors in integration tests + allowlist docstring - packages_update_test: add missing lockKey arg to registry.Apply - mcp_grant_revoke_test: remove unused fakeMCPClient struct - allowlist.go: fix Check() docstring to match actual 3-step pipeline * fix(test): relax mcp grant revoke assertion for pre-Phase02 state Execute-time grant checking not yet wired — test correctly gets an error but the message is "no active client" (nil clientPtr) rather than "grant revoked". Accept any error as valid regression guard. * chore: trigger CI on digitopvn/goclaw fork * ci: retrigger workflows * fix(permissions): classify workstation methods in RBAC policy
354 lines
12 KiB
Go
354 lines
12 KiB
Go
package skills
|
|
|
|
import (
|
|
"context"
|
|
"encoding/json"
|
|
"errors"
|
|
"fmt"
|
|
"io"
|
|
"log/slog"
|
|
"net/http"
|
|
"net/url"
|
|
"strconv"
|
|
"strings"
|
|
"sync"
|
|
"time"
|
|
)
|
|
|
|
// Sentinel errors returned by the GitHub API client.
|
|
var (
|
|
ErrGitHubNotFound = errors.New("github: release not found")
|
|
ErrGitHubUnauthorized = errors.New("github: unauthorized (check token)")
|
|
ErrGitHubRateLimited = errors.New("github: rate limited")
|
|
ErrGitHubServer = errors.New("github: server error")
|
|
)
|
|
|
|
// GitHubAsset describes a single release asset.
|
|
type GitHubAsset struct {
|
|
Name string `json:"name"`
|
|
DownloadURL string `json:"browser_download_url"`
|
|
SizeBytes int64 `json:"size"`
|
|
ContentType string `json:"content_type"`
|
|
}
|
|
|
|
// GitHubRelease is a simplified projection of the GitHub release payload.
|
|
type GitHubRelease struct {
|
|
TagName string `json:"tag_name"`
|
|
Name string `json:"name"`
|
|
PublishedAt time.Time `json:"published_at"`
|
|
Prerelease bool `json:"prerelease"`
|
|
Draft bool `json:"draft"`
|
|
Assets []GitHubAsset `json:"assets"`
|
|
}
|
|
|
|
// releaseCacheEntry is a single cached release lookup.
|
|
type releaseCacheEntry struct {
|
|
data any
|
|
expiresAt time.Time
|
|
}
|
|
|
|
// GitHubClient is a minimal REST client for the GitHub Releases API.
|
|
// Supports optional bearer token (private repos + higher rate limit) and
|
|
// an in-memory 10-minute TTL cache keyed by "owner/repo:tag".
|
|
type GitHubClient struct {
|
|
Token string
|
|
BaseURL string // default "https://api.github.com" — overridable for tests
|
|
HTTPClient *http.Client
|
|
|
|
mu sync.Mutex
|
|
cache map[string]releaseCacheEntry
|
|
ttl time.Duration
|
|
}
|
|
|
|
// NewGitHubClient creates a client. If httpClient is nil, a default with 30s timeout is used.
|
|
func NewGitHubClient(token string) *GitHubClient {
|
|
return &GitHubClient{
|
|
Token: token,
|
|
BaseURL: "https://api.github.com",
|
|
HTTPClient: &http.Client{Timeout: 30 * time.Second},
|
|
cache: make(map[string]releaseCacheEntry),
|
|
ttl: 10 * time.Minute,
|
|
}
|
|
}
|
|
|
|
func (c *GitHubClient) cacheGet(key string) (any, bool) {
|
|
c.mu.Lock()
|
|
defer c.mu.Unlock()
|
|
e, ok := c.cache[key]
|
|
if !ok || time.Now().After(e.expiresAt) {
|
|
return nil, false
|
|
}
|
|
return e.data, true
|
|
}
|
|
|
|
// cacheMaxEntries is a SOFT sweep trigger, not a hard cap: once the map
|
|
// reaches this size we scan for expired entries and drop them before
|
|
// inserting the new one. If every entry is still live the map can briefly
|
|
// exceed the threshold — in practice the 10-minute TTL keeps growth bounded
|
|
// by the request rate. Prevents unbounded growth from many distinct repos
|
|
// being queried over long uptime.
|
|
const cacheMaxEntries = 256
|
|
|
|
func (c *GitHubClient) cacheSet(key string, v any) {
|
|
c.mu.Lock()
|
|
defer c.mu.Unlock()
|
|
if len(c.cache) >= cacheMaxEntries {
|
|
now := time.Now()
|
|
for k, e := range c.cache {
|
|
if now.After(e.expiresAt) {
|
|
delete(c.cache, k)
|
|
}
|
|
}
|
|
}
|
|
c.cache[key] = releaseCacheEntry{data: v, expiresAt: time.Now().Add(c.ttl)}
|
|
}
|
|
|
|
// GetRelease fetches a single release by tag. If tag is empty, "latest" is used.
|
|
func (c *GitHubClient) GetRelease(ctx context.Context, owner, repo, tag string) (*GitHubRelease, error) {
|
|
key := fmt.Sprintf("rel:%s/%s:%s", owner, repo, tag)
|
|
if v, ok := c.cacheGet(key); ok {
|
|
r := v.(*GitHubRelease)
|
|
return r, nil
|
|
}
|
|
|
|
var path string
|
|
if tag == "" {
|
|
path = fmt.Sprintf("/repos/%s/%s/releases/latest",
|
|
url.PathEscape(owner), url.PathEscape(repo))
|
|
} else {
|
|
// PathEscape the tag so characters valid in git refs but URL-special
|
|
// (#, ?, %, +) don't silently corrupt the path (# → fragment, ? → query).
|
|
path = fmt.Sprintf("/repos/%s/%s/releases/tags/%s",
|
|
url.PathEscape(owner), url.PathEscape(repo), url.PathEscape(tag))
|
|
}
|
|
|
|
var rel GitHubRelease
|
|
if err := c.doJSON(ctx, path, &rel); err != nil {
|
|
return nil, err
|
|
}
|
|
c.cacheSet(key, &rel)
|
|
return &rel, nil
|
|
}
|
|
|
|
// ListReleases returns the most recent releases (at most `limit`, max 100).
|
|
func (c *GitHubClient) ListReleases(ctx context.Context, owner, repo string, limit int) ([]GitHubRelease, error) {
|
|
if limit <= 0 {
|
|
limit = 10
|
|
}
|
|
if limit > 100 {
|
|
limit = 100
|
|
}
|
|
key := fmt.Sprintf("list:%s/%s:%d", owner, repo, limit)
|
|
if v, ok := c.cacheGet(key); ok {
|
|
return v.([]GitHubRelease), nil
|
|
}
|
|
path := fmt.Sprintf("/repos/%s/%s/releases?per_page=%d",
|
|
url.PathEscape(owner), url.PathEscape(repo), limit)
|
|
var releases []GitHubRelease
|
|
if err := c.doJSON(ctx, path, &releases); err != nil {
|
|
return nil, err
|
|
}
|
|
c.cacheSet(key, releases)
|
|
return releases, nil
|
|
}
|
|
|
|
// ErrGitHubSecondaryRateLimit is returned when GitHub signals a secondary
|
|
// (abuse-detection) rate limit via 403 + Retry-After. The header value is
|
|
// embedded in the error's Error() message; callers may inspect via the
|
|
// SecondaryRateLimit type assertion.
|
|
var ErrGitHubSecondaryRateLimit = errors.New("github: secondary rate limit (Retry-After)")
|
|
|
|
// CondGetRelease fetches a release with If-None-Match support.
|
|
//
|
|
// tag=="" → /releases/latest
|
|
// tag!="" → /releases/tags/{tag}
|
|
//
|
|
// Returns release==nil AND notModified=true on 304 (no body). Otherwise
|
|
// populates release and newETag. Errors map to the same sentinels as
|
|
// GetRelease. Does NOT consult the 10-minute cache (ETag is the cache now).
|
|
func (c *GitHubClient) CondGetRelease(ctx context.Context, owner, repo, tag, ifNoneMatch string) (rel *GitHubRelease, newETag string, notModified bool, err error) {
|
|
var path string
|
|
if tag == "" {
|
|
path = fmt.Sprintf("/repos/%s/%s/releases/latest",
|
|
url.PathEscape(owner), url.PathEscape(repo))
|
|
} else {
|
|
path = fmt.Sprintf("/repos/%s/%s/releases/tags/%s",
|
|
url.PathEscape(owner), url.PathEscape(repo), url.PathEscape(tag))
|
|
}
|
|
var out GitHubRelease
|
|
etag, mod, err := c.doJSONConditional(ctx, path, ifNoneMatch, &out)
|
|
if err != nil {
|
|
return nil, "", false, err
|
|
}
|
|
if mod {
|
|
return nil, etag, true, nil
|
|
}
|
|
return &out, etag, false, nil
|
|
}
|
|
|
|
// CondListReleases fetches up to `limit` recent releases with If-None-Match
|
|
// support. Returns nil slice AND notModified=true on 304.
|
|
func (c *GitHubClient) CondListReleases(ctx context.Context, owner, repo string, limit int, ifNoneMatch string) (rels []GitHubRelease, newETag string, notModified bool, err error) {
|
|
if limit <= 0 {
|
|
limit = 10
|
|
}
|
|
if limit > 100 {
|
|
limit = 100
|
|
}
|
|
path := fmt.Sprintf("/repos/%s/%s/releases?per_page=%d",
|
|
url.PathEscape(owner), url.PathEscape(repo), limit)
|
|
var out []GitHubRelease
|
|
etag, mod, err := c.doJSONConditional(ctx, path, ifNoneMatch, &out)
|
|
if err != nil {
|
|
return nil, "", false, err
|
|
}
|
|
if mod {
|
|
return nil, etag, true, nil
|
|
}
|
|
return out, etag, false, nil
|
|
}
|
|
|
|
// doJSONConditional performs a GET with optional If-None-Match.
|
|
// Returns (newETag, notModified, err).
|
|
//
|
|
// Secondary rate limits: GitHub returns 403 with Retry-After header and
|
|
// zero X-RateLimit-Remaining; this path maps to ErrGitHubSecondaryRateLimit
|
|
// when Retry-After is present, preserving the hint via fmt.Errorf wrapping.
|
|
func (c *GitHubClient) doJSONConditional(ctx context.Context, path, ifNoneMatch string, out any) (string, bool, error) {
|
|
apiURL := c.BaseURL + path
|
|
req, err := http.NewRequestWithContext(ctx, http.MethodGet, apiURL, nil)
|
|
if err != nil {
|
|
return "", false, err
|
|
}
|
|
req.Header.Set("Accept", "application/vnd.github+json")
|
|
req.Header.Set("X-GitHub-Api-Version", "2022-11-28")
|
|
if c.Token != "" {
|
|
req.Header.Set("Authorization", "Bearer "+c.Token)
|
|
}
|
|
if ifNoneMatch != "" {
|
|
req.Header.Set("If-None-Match", ifNoneMatch)
|
|
}
|
|
|
|
resp, err := c.HTTPClient.Do(req)
|
|
if err != nil {
|
|
return "", false, fmt.Errorf("github: http request failed: %w", err)
|
|
}
|
|
defer resp.Body.Close()
|
|
|
|
// 304 Not Modified — body empty, preserve the ETag we sent (GitHub repeats
|
|
// it in the response header for consistency).
|
|
if resp.StatusCode == http.StatusNotModified {
|
|
etag := resp.Header.Get("ETag")
|
|
if etag == "" {
|
|
etag = ifNoneMatch
|
|
}
|
|
return etag, true, nil
|
|
}
|
|
|
|
switch {
|
|
case resp.StatusCode == http.StatusOK:
|
|
// fall through
|
|
case resp.StatusCode == http.StatusNotFound:
|
|
return "", false, ErrGitHubNotFound
|
|
case resp.StatusCode == http.StatusUnauthorized:
|
|
return "", false, ErrGitHubUnauthorized
|
|
case resp.StatusCode == http.StatusForbidden:
|
|
// Secondary rate limit (abuse detection) — identifiable by Retry-After.
|
|
if ra := resp.Header.Get("Retry-After"); ra != "" {
|
|
return "", false, fmt.Errorf("%w (retry_after=%s)", ErrGitHubSecondaryRateLimit, ra)
|
|
}
|
|
remaining := resp.Header.Get("X-RateLimit-Remaining")
|
|
if remaining == "0" {
|
|
reset := resp.Header.Get("X-RateLimit-Reset")
|
|
if n, errConv := strconv.ParseInt(reset, 10, 64); errConv == nil {
|
|
return "", false, fmt.Errorf("%w (resets at %s)", ErrGitHubRateLimited, time.Unix(n, 0).UTC().Format(time.RFC3339))
|
|
}
|
|
return "", false, ErrGitHubRateLimited
|
|
}
|
|
return "", false, ErrGitHubUnauthorized
|
|
case resp.StatusCode == http.StatusTooManyRequests:
|
|
return "", false, ErrGitHubRateLimited
|
|
case resp.StatusCode >= 500:
|
|
return "", false, ErrGitHubServer
|
|
default:
|
|
body, _ := io.ReadAll(io.LimitReader(resp.Body, 1024))
|
|
return "", false, fmt.Errorf("github: unexpected status %d: %s", resp.StatusCode, strings.TrimSpace(string(body)))
|
|
}
|
|
|
|
const maxAPIResponseBytes = 8 * 1024 * 1024
|
|
if err := json.NewDecoder(io.LimitReader(resp.Body, maxAPIResponseBytes)).Decode(out); err != nil {
|
|
return "", false, fmt.Errorf("github: decode response: %w", err)
|
|
}
|
|
// Warn on low rate limit remaining.
|
|
if rem := resp.Header.Get("X-RateLimit-Remaining"); rem != "" {
|
|
if n, errConv := strconv.Atoi(rem); errConv == nil && n < 5 {
|
|
slog.Warn("security.github.ratelimit.low",
|
|
"remaining", n, "reset", resp.Header.Get("X-RateLimit-Reset"))
|
|
}
|
|
}
|
|
return resp.Header.Get("ETag"), false, nil
|
|
}
|
|
|
|
// doJSON performs a GET + JSON decode, mapping status codes to sentinel errors.
|
|
func (c *GitHubClient) doJSON(ctx context.Context, path string, out any) error {
|
|
// Avoid shadowing the "net/url" package import used elsewhere in this file.
|
|
apiURL := c.BaseURL + path
|
|
req, err := http.NewRequestWithContext(ctx, http.MethodGet, apiURL, nil)
|
|
if err != nil {
|
|
return err
|
|
}
|
|
req.Header.Set("Accept", "application/vnd.github+json")
|
|
req.Header.Set("X-GitHub-Api-Version", "2022-11-28")
|
|
if c.Token != "" {
|
|
req.Header.Set("Authorization", "Bearer "+c.Token)
|
|
}
|
|
|
|
resp, err := c.HTTPClient.Do(req)
|
|
if err != nil {
|
|
return fmt.Errorf("github: http request failed: %w", err)
|
|
}
|
|
defer resp.Body.Close()
|
|
|
|
switch {
|
|
case resp.StatusCode == http.StatusOK:
|
|
// fall through
|
|
case resp.StatusCode == http.StatusNotFound:
|
|
return ErrGitHubNotFound
|
|
case resp.StatusCode == http.StatusUnauthorized:
|
|
return ErrGitHubUnauthorized
|
|
case resp.StatusCode == http.StatusForbidden:
|
|
// Rate limit check
|
|
remaining := resp.Header.Get("X-RateLimit-Remaining")
|
|
if remaining == "0" {
|
|
reset := resp.Header.Get("X-RateLimit-Reset")
|
|
if n, errConv := strconv.ParseInt(reset, 10, 64); errConv == nil {
|
|
return fmt.Errorf("%w (resets at %s)", ErrGitHubRateLimited, time.Unix(n, 0).UTC().Format(time.RFC3339))
|
|
}
|
|
return ErrGitHubRateLimited
|
|
}
|
|
return ErrGitHubUnauthorized
|
|
case resp.StatusCode == http.StatusTooManyRequests:
|
|
// GitHub secondary rate limits (abuse detection, search, unauthenticated
|
|
// bursts) return 429 rather than 403+X-RateLimit-Remaining:0. Map both
|
|
// onto the same sentinel so the HTTP handler renders a 429 "rate limit
|
|
// reached" instead of a 502 "failed to fetch releases".
|
|
return ErrGitHubRateLimited
|
|
case resp.StatusCode >= 500:
|
|
return ErrGitHubServer
|
|
default:
|
|
body, _ := io.ReadAll(io.LimitReader(resp.Body, 1024))
|
|
return fmt.Errorf("github: unexpected status %d: %s", resp.StatusCode, strings.TrimSpace(string(body)))
|
|
}
|
|
|
|
// Cap the response body at a generous 8 MiB. GitHub's release/list
|
|
// payloads are well under this (a 100-release list with rich asset
|
|
// metadata sits around 1 MiB). Belt-and-braces in case a future caller
|
|
// adds a path that could return a much larger document, or a
|
|
// man-in-the-middle / misbehaving upstream sends an oversized body.
|
|
const maxAPIResponseBytes = 8 * 1024 * 1024
|
|
if err := json.NewDecoder(io.LimitReader(resp.Body, maxAPIResponseBytes)).Decode(out); err != nil {
|
|
return fmt.Errorf("github: decode response: %w", err)
|
|
}
|
|
return nil
|
|
}
|