From 4780307e0b1c60dc759724bf960b86bc23522000 Mon Sep 17 00:00:00 2001 From: Ameen Rashad <7609895+vazra@users.noreply.github.com> Date: Sun, 11 Oct 2026 01:47:23 -0700 Subject: [PATCH 1/6] fix(api): keep stop/start/scale/rollback running when the client disconnects Run these compose operations under a context detached from the request (context.WithoutCancel + 30 min timeout) so closing the tab no longer kills docker compose mid-way. Audit rows use the same context. --- internal/api/actions.go | 37 ++++++++--- internal/api/actions_test.go | 115 +++++++++++++++++++++++++++++++++++ 2 files changed, 144 insertions(+), 8 deletions(-) diff --git a/internal/api/actions.go b/internal/api/actions.go index e3a51fa4..17df1dcb 100644 --- a/internal/api/actions.go +++ b/internal/api/actions.go @@ -10,6 +10,7 @@ import ( "os" "path/filepath" "strconv" + "time" "github.com/vazra/simpledeploy/internal/audit" "github.com/vazra/simpledeploy/internal/compose" @@ -28,6 +29,18 @@ type serviceInfo struct { ScaleReason string `json:"scale_reason,omitempty"` } +// composeOpTimeout bounds a docker compose operation that runs detached +// from its request, so a hung daemon cannot hold it forever. +const composeOpTimeout = 30 * time.Minute + +// detachedCtx returns a context for a docker compose operation that must +// finish even if the client goes away. Closing the tab cancels r.Context(), +// which would kill the compose process mid-way (e.g. leave 1 of 2 replicas). +// Request values (actor for audit) are kept. +func detachedCtx(r *http.Request) (context.Context, context.CancelFunc) { + return context.WithTimeout(context.WithoutCancel(r.Context()), composeOpTimeout) +} + func (s *Server) handleRestart(w http.ResponseWriter, r *http.Request) { slug := r.PathValue("slug") if s.reconciler == nil { @@ -58,12 +71,14 @@ func (s *Server) handleStop(w http.ResponseWriter, r *http.Request) { http.Error(w, "reconciler not configured", http.StatusInternalServerError) return } - if err := s.reconciler.StopOne(r.Context(), slug); err != nil { + ctx, cancel := detachedCtx(r) + defer cancel() + if err := s.reconciler.StopOne(ctx, slug); err != nil { httpError(w, err, http.StatusInternalServerError) return } afterJSON, _ := json.Marshal(map[string]any{"name": slug, "status": "stopped"}) - _, _ = s.audit.Record(r.Context(), audit.RecordReq{ + _, _ = s.audit.Record(ctx, audit.RecordReq{ Category: "lifecycle", Action: "stopped", AppSlug: slug, @@ -79,12 +94,14 @@ func (s *Server) handleStart(w http.ResponseWriter, r *http.Request) { http.Error(w, "reconciler not configured", http.StatusInternalServerError) return } - if err := s.reconciler.StartOne(r.Context(), slug); err != nil { + ctx, cancel := detachedCtx(r) + defer cancel() + if err := s.reconciler.StartOne(ctx, slug); err != nil { httpError(w, err, http.StatusInternalServerError) return } afterJSON, _ := json.Marshal(map[string]any{"name": slug, "status": "running"}) - _, _ = s.audit.Record(r.Context(), audit.RecordReq{ + _, _ = s.audit.Record(ctx, audit.RecordReq{ Category: "lifecycle", Action: "started", AppSlug: slug, @@ -174,7 +191,9 @@ func (s *Server) handleScale(w http.ResponseWriter, r *http.Request) { } } - if err := s.reconciler.ScaleOne(r.Context(), slug, body.Scales); err != nil { + ctx, cancel := detachedCtx(r) + defer cancel() + if err := s.reconciler.ScaleOne(ctx, slug, body.Scales); err != nil { // Forward the actual error (which wraps docker compose stderr) so the // UI can show something more useful than "Internal Server Error". log.Printf("[api] scale %s: %v", slug, err) @@ -184,7 +203,7 @@ func (s *Server) handleScale(w http.ResponseWriter, r *http.Request) { beforeJSON, _ := json.Marshal(map[string]any{"name": slug, "replicas": beforeReplicas}) afterJSON, _ := json.Marshal(map[string]any{"name": slug, "replicas": afterReplicas}) - _, _ = s.audit.Record(r.Context(), audit.RecordReq{ + _, _ = s.audit.Record(ctx, audit.RecordReq{ Category: "lifecycle", Action: "scaled", AppSlug: slug, @@ -346,7 +365,9 @@ func (s *Server) handleRollback(w http.ResponseWriter, r *http.Request) { if _, ok := s.checkVersionForRestore(w, r, slug, filepath.Join(s.appsDir, slug, "docker-compose.yml"), ver); !ok { return } - if err := s.reconciler.RollbackOne(r.Context(), slug, body.VersionID); err != nil { + ctx, cancel := detachedCtx(r) + defer cancel() + if err := s.reconciler.RollbackOne(ctx, slug, body.VersionID); err != nil { if v, ok := violationsOf(err); ok { writeViolations(w, http.StatusBadRequest, versionRefused, v) return @@ -359,7 +380,7 @@ func (s *Server) handleRollback(w http.ResponseWriter, r *http.Request) { return } afterJSON, _ := json.Marshal(map[string]any{"version_id": body.VersionID}) - _, _ = s.audit.Record(r.Context(), audit.RecordReq{ + _, _ = s.audit.Record(ctx, audit.RecordReq{ Category: "lifecycle", Action: "rolled_back", AppSlug: slug, diff --git a/internal/api/actions_test.go b/internal/api/actions_test.go index d8faadaa..88654ca9 100644 --- a/internal/api/actions_test.go +++ b/internal/api/actions_test.go @@ -7,7 +7,10 @@ import ( "fmt" "net/http" "net/http/httptest" + "os" + "path/filepath" "testing" + "time" "github.com/vazra/simpledeploy/internal/compose" "github.com/vazra/simpledeploy/internal/deployer" @@ -215,3 +218,115 @@ func TestScaleEndpointMissingBody(t *testing.T) { t.Fatalf("status = %d, want 400", w.Code) } } + +// blockingRunner is a deployer.CommandRunner that hands each compose call's +// context to the test and blocks until released. Like exec.CommandContext, +// it fails if that context is canceled. +type blockingRunner struct { + ctxs chan context.Context + release chan struct{} +} + +func (b *blockingRunner) Run(ctx context.Context, _ string, args ...string) (string, string, error) { + if len(args) == 2 && args[1] == "version" { + return "", "", nil + } + b.ctxs <- ctx + select { + case <-b.release: + case <-ctx.Done(): + } + return "", "", ctx.Err() +} + +// runnerReconciler routes lifecycle actions through a real deployer so the +// context reaching the CommandRunner is the one the handler passed. +type runnerReconciler struct { + mockReconcilerFull + d *deployer.Deployer + runner deployer.CommandRunner + composePath string +} + +func (m *runnerReconciler) StopOne(ctx context.Context, slug string) error { + return m.d.Stop(ctx, slug) +} +func (m *runnerReconciler) StartOne(ctx context.Context, slug string) error { + return m.d.Start(ctx, slug) +} +func (m *runnerReconciler) ScaleOne(ctx context.Context, slug string, scales map[string]int) error { + return m.d.Scale(ctx, &compose.AppConfig{Name: slug, ComposePath: m.composePath}, scales) +} +func (m *runnerReconciler) RollbackOne(ctx context.Context, _ string, _ int64) error { + _, _, err := m.runner.Run(ctx, "docker", "compose", "up", "-d") + return err +} + +func TestLifecycleActionsSurviveClientDisconnect(t *testing.T) { + scale, _ := json.Marshal(map[string]any{"scales": map[string]int{"web": 2}}) + cases := []struct { + name, path string + body []byte + rollback bool + }{ + {"stop", "/api/apps/myapp/stop", nil, false}, + {"start", "/api/apps/myapp/start", nil, false}, + {"scale", "/api/apps/myapp/scale", scale, false}, + {"rollback", "/api/apps/myapp/rollback", nil, true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + srv, _ := newTestServer(t) + srv.SetAppsDir(t.TempDir()) + runner := &blockingRunner{ctxs: make(chan context.Context, 1), release: make(chan struct{})} + d, err := deployer.New(runner) + if err != nil { + t.Fatal(err) + } + composePath := filepath.Join(t.TempDir(), "docker-compose.yml") + if err := os.WriteFile(composePath, []byte("services:\n web:\n image: nginx\n"), 0o644); err != nil { + t.Fatal(err) + } + srv.SetReconciler(&runnerReconciler{d: d, runner: runner, composePath: composePath}) + + body := tc.body + if tc.rollback { + body, _ = json.Marshal(map[string]any{"version_id": seedVersion(t, srv, "myapp")}) + } + reqCtx, closeTab := context.WithCancel(context.Background()) + defer closeTab() + req := httptest.NewRequest(http.MethodPost, tc.path, bytes.NewReader(body)).WithContext(reqCtx) + req.Header.Set("Content-Type", "application/json") + req.AddCookie(superAdminCookie(t, srv.jwt)) + w := httptest.NewRecorder() + done := make(chan struct{}) + go func() { + srv.Handler().ServeHTTP(w, req) + close(done) + }() + + var opCtx context.Context + select { + case opCtx = <-runner.ctxs: + case <-time.After(5 * time.Second): + t.Fatal("compose command never ran") + } + closeTab() + if err := opCtx.Err(); err != nil { + t.Fatalf("compose command context canceled with the request: %v", err) + } + if _, ok := opCtx.Deadline(); !ok { + t.Error("compose command context has no timeout") + } + close(runner.release) + select { + case <-done: + case <-time.After(5 * time.Second): + t.Fatal("handler did not return") + } + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body: %s", w.Code, w.Body.String()) + } + }) + } +} From f1b989072a2adb4d49a012fca8b62e344e27948f Mon Sep 17 00:00:00 2001 From: Ameen Rashad <7609895+vazra@users.noreply.github.com> Date: Sun, 11 Oct 2026 01:48:17 -0700 Subject: [PATCH 2/6] fix(ui): describe restart as an in-place restart, not a recreate The backend runs docker compose restart. Also correct the restart/pull API docs (202 started) and note that sync actions survive disconnects. --- docs/reference/api.md | 6 ++++-- ui/src/routes/AppDetail.svelte | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/reference/api.md b/docs/reference/api.md index 4e89ebc2..be2691e4 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -135,9 +135,11 @@ Set the IP allowlist. Body: `{"allow": "1.2.3.4, 10.0.0.0/8"}` (empty string all ## App Actions +Stop, start, scale and rollback wait for `docker compose` to finish before responding. The operation keeps running if the client disconnects mid-request. + ### `POST /api/apps/{slug}/restart` -Restart all containers for the app. Returns `{"status": "ok"}`. +Restart the app's existing containers in place (`docker compose restart`). Containers are not recreated, so compose changes and newer images are not picked up (use pull or a deploy for that). Runs in the background and returns `202` with `{"status": "started"}`. ### `POST /api/apps/{slug}/stop` @@ -149,7 +151,7 @@ Start stopped containers. Returns `{"status": "ok"}`. ### `POST /api/apps/{slug}/pull` -Pull latest images and redeploy. Uses configured registry auth if available. Returns `{"status": "ok"}`. +Pull latest images and redeploy. Uses configured registry auth if available. Runs in the background and returns `202` with `{"status": "started"}`. ### `POST /api/apps/{slug}/scale` diff --git a/ui/src/routes/AppDetail.svelte b/ui/src/routes/AppDetail.svelte index 632c22e7..35ae069d 100644 --- a/ui/src/routes/AppDetail.svelte +++ b/ui/src/routes/AppDetail.svelte @@ -452,7 +452,7 @@

Restart App

-

This will force-recreate all containers for {app.Name}. Continue?

+

Each service in {app.Name} will be stopped and started again, keeping its current settings and data. The app may be unavailable for a few seconds. To download newer images, use Pull & Update instead.

From 209f182dea87da2c899de53463f40d9c1b68ca58 Mon Sep 17 00:00:00 2001 From: Ameen Rashad <7609895+vazra@users.noreply.github.com> Date: Sun, 11 Oct 2026 01:49:37 -0700 Subject: [PATCH 3/6] fix(ui): stay on the app page when deleting it fails confirmDelete navigated to the app list even when removeApp returned an error. Show the reason inline and only navigate on success. --- ui/src/components/SettingsTab.svelte | 15 +++++++++- .../components/__tests__/SettingsTab.test.js | 28 +++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/ui/src/components/SettingsTab.svelte b/ui/src/components/SettingsTab.svelte index b56e7284..f58f5ab9 100644 --- a/ui/src/components/SettingsTab.svelte +++ b/ui/src/components/SettingsTab.svelte @@ -53,6 +53,7 @@ let showDanger = $state(false) let showDeleteModal = $state(false) let deleting = $state(false) + let deleteError = $state('') // Export let exporting = $state(false) @@ -190,9 +191,16 @@ async function confirmDelete() { deleting = true - await api.removeApp(slug) + deleteError = '' + const res = await api.removeApp(slug) deleting = false showDeleteModal = false + // On failure the API call already toasted the reason; stay on the page + // so it is clear the app still exists. + if (res?.error) { + deleteError = res.error + return + } push('/') } @@ -480,6 +488,11 @@ Delete App
+ {#if deleteError} + + {/if}
{/if} diff --git a/ui/src/components/__tests__/SettingsTab.test.js b/ui/src/components/__tests__/SettingsTab.test.js index f0c9387b..e488bd7f 100644 --- a/ui/src/components/__tests__/SettingsTab.test.js +++ b/ui/src/components/__tests__/SettingsTab.test.js @@ -9,6 +9,7 @@ vi.mock('svelte-spa-router', () => ({ default: null, })); +import { push } from 'svelte-spa-router'; import SettingsTab from '../SettingsTab.svelte'; import { toasts } from '../../lib/stores/toast.js'; @@ -82,6 +83,7 @@ describe('SettingsTab', () => { routes = {}; installFetch(); for (const t of get(toasts)) toasts.remove(t.id); + push.mockClear(); }); afterEach(() => { @@ -194,6 +196,32 @@ describe('SettingsTab', () => { expect(getByLabelText('IP Allowlist').value).toBe('10.0.0.0/8'); }); + async function deleteApp() { + const r = renderTab(); + await fireEvent.click(r.getByRole('button', { name: /Danger Zone/i })); + await fireEvent.click(r.getByRole('button', { name: /Delete App/i })); + await fireEvent.click(r.getByRole('button', { name: 'Confirm' })); + return r; + } + + it('stays on the page and shows the error when deleting the app fails', async () => { + routes['DELETE /api/apps/foo'] = () => json({ error: 'compose down failed: daemon unreachable' }, 500); + const { findByTestId } = await deleteApp(); + + await waitFor(() => expect(calls('DELETE', '/api/apps/foo')).toHaveLength(1)); + expect((await findByTestId('delete-error')).textContent).toMatch(/daemon unreachable/); + expect(get(toasts).some((t) => t.type === 'error' && /daemon unreachable/.test(t.message))).toBe(true); + expect(push).not.toHaveBeenCalled(); + }); + + it('goes back to the app list after the app is deleted', async () => { + routes['DELETE /api/apps/foo'] = () => json({ status: 'ok' }); + const { queryByTestId } = await deleteApp(); + + await waitFor(() => expect(push).toHaveBeenCalledWith('/')); + expect(queryByTestId('delete-error')).toBeNull(); + }); + it('gives viewers a read-only environment editor', async () => { const { findByTestId, queryByText } = renderTab({ canMutate: false }); expect(await findByTestId('env-readonly-hint')).toBeInTheDocument(); From 59736373d9bd3b0a9e98f6a04e5ba12575875dbc Mon Sep 17 00:00:00 2001 From: Ameen Rashad <7609895+vazra@users.noreply.github.com> Date: Sun, 11 Oct 2026 01:53:31 -0700 Subject: [PATCH 4/6] fix(proxy): honor simpledeploy.ratelimit.burst The label was parsed but ignored. Once a window's requests are used up, up to burst more pass; the burst refills only after a whole window within requests, so sustained traffic stays at the steady rate. burst 0 (label unset) keeps the plain fixed window. Docs: correct label defaults and mark config.yaml ratelimit.burst/by as ignored (management API only). --- docs/architecture/auth.md | 2 +- docs/architecture/proxy.md | 2 +- docs/guides/rate-limiting.md | 8 ++--- docs/guides/small-vps-tuning.md | 3 +- docs/reference/compose-labels.md | 8 ++--- docs/reference/configuration.md | 13 ++++---- internal/proxy/ratelimit.go | 30 +++++++++++++----- internal/proxy/ratelimit_test.go | 52 ++++++++++++++++++++++++++++++++ 8 files changed, 91 insertions(+), 27 deletions(-) diff --git a/docs/architecture/auth.md b/docs/architecture/auth.md index bb12e219..1db1d77e 100644 --- a/docs/architecture/auth.md +++ b/docs/architecture/auth.md @@ -31,7 +31,7 @@ API keys inherit the creator's role and per-app grants at the moment of creation ## Rate limiting -A per-IP token bucket limits authentication attempts and dashboard requests. Defaults: 200 requests per 60 seconds with a burst of 50, keyed by IP. Configurable in `config.yaml`. Apps can override per-route via compose labels. +A per-IP token bucket limits authentication attempts and dashboard requests. Defaults: 200 requests per 60 seconds, keyed by IP. Configurable in `config.yaml` (`ratelimit.burst` and `ratelimit.by` are ignored here). Proxied app traffic has its own per-app limits from compose labels. A separate, stricter limit on `/api/auth/login` blocks credential stuffing. After repeated failures, the account is temporarily locked. diff --git a/docs/architecture/proxy.md b/docs/architecture/proxy.md index 46be6b7d..5ee9bcca 100644 --- a/docs/architecture/proxy.md +++ b/docs/architecture/proxy.md @@ -56,7 +56,7 @@ All three are registered in `init()` of their respective files in [/internal/pro ### `simpledeploy_ratelimit` -[/internal/proxy/ratelimit.go](https://github.com/vazra/simpledeploy/blob/main/internal/proxy/ratelimit.go). Per-domain configs registered via `RateLimiters.Set(domain, cfg)` from `SetRoutes`. Each domain has its own `domainLimiter` keyed by client IP (default), header value, or query param depending on `simpledeploy.ratelimit.by`. Each domain keeps at most 10,000 buckets: when full, buckets idle for a whole window are evicted first, then the least recently used. Keys longer than 128 bytes are stored as their SHA-256. +[/internal/proxy/ratelimit.go](https://github.com/vazra/simpledeploy/blob/main/internal/proxy/ratelimit.go). Per-domain configs registered via `RateLimiters.Set(domain, cfg)` from `SetRoutes`. Each domain has its own `domainLimiter` keyed by client IP (default), header value, or path depending on `simpledeploy.ratelimit.by`. Each bucket allows `requests` per window; a window starts at the bucket's first request after the previous one ended. With `burst` > 0, up to `burst` more requests pass once the window's `requests` are used up. The burst refills only after a whole window within `requests` (or with no requests), so a client that keeps going over is held to `requests` per window. `burst` 0 (label unset) is a plain fixed window. Each domain keeps at most 10,000 buckets: when full, buckets idle for a whole window are evicted first, then the least recently used. Keys longer than 128 bytes are stored as their SHA-256. ### `simpledeploy_ipaccess` diff --git a/docs/guides/rate-limiting.md b/docs/guides/rate-limiting.md index 2066b535..666cf246 100644 --- a/docs/guides/rate-limiting.md +++ b/docs/guides/rate-limiting.md @@ -9,12 +9,12 @@ Rate limiting is per-app, configured with `simpledeploy.ratelimit.*` labels on a | Label | Default | Description | |-------|---------|-------------| -| `simpledeploy.ratelimit.requests` | `200` (server config) | Allowed requests per window | +| `simpledeploy.ratelimit.requests` | none | Allowed requests per window. Rate limiting is on only when this is set | | `simpledeploy.ratelimit.window` | `60s` | Length of the window | -| `simpledeploy.ratelimit.burst` | `50` | Extra short-spike allowance over the steady rate | +| `simpledeploy.ratelimit.burst` | `0` | Extra requests allowed in a short spike, on top of `requests` | | `simpledeploy.ratelimit.by` | `ip` | Key to bucket by: `ip`, `header:NAME`, or `path` | -`requests`/`window` define the steady rate. `burst` lets a client briefly exceed it before getting throttled. Once the bucket is empty, clients get `429 Too Many Requests` with a `Retry-After: 60` header. +`requests`/`window` define the steady rate. `burst` lets a client briefly go over it: once a window's `requests` are used up, up to `burst` more get through. A client that keeps going over is held to `requests` per window; the burst comes back after it stays within `requests` (or sends nothing) for a whole window. Without `burst`, the limit is exactly `requests` per window. Once nothing is left, clients get `429 Too Many Requests` with a `Retry-After: 60` header. ## Per-IP (most common) @@ -31,7 +31,7 @@ services: simpledeploy.ratelimit.by: "ip" ``` -100 req/min/IP, with burst up to 120. +100 req/min/IP. A short spike can reach 120 in one minute; a client that keeps going over is held to 100 a minute. ## Per API key (header) diff --git a/docs/guides/small-vps-tuning.md b/docs/guides/small-vps-tuning.md index 80c2526b..d60fa6a1 100644 --- a/docs/guides/small-vps-tuning.md +++ b/docs/guides/small-vps-tuning.md @@ -45,10 +45,9 @@ Defaults assume a beefier host. On 1 vCPU: ratelimit: requests: 100 window: 60s - burst: 25 ``` -This blocks bursty scrapers that would otherwise pin CPU. +This limits requests to the dashboard and management API. To slow down scrapers hitting your apps, add [rate limit labels](/guides/rate-limiting/) to those apps. ## Prune Docker regularly diff --git a/docs/reference/compose-labels.md b/docs/reference/compose-labels.md index d50a359f..035a5ab8 100644 --- a/docs/reference/compose-labels.md +++ b/docs/reference/compose-labels.md @@ -226,10 +226,10 @@ See [Rate limiting](/guides/rate-limiting/) for the full guide, including key ty | Label | Default | Description | |-------|---------|-------------| -| `simpledeploy.ratelimit.requests` | global default (200) | Max requests per window | -| `simpledeploy.ratelimit.window` | global default (60s) | Time window | -| `simpledeploy.ratelimit.burst` | global default (50) | Burst allowance above limit | -| `simpledeploy.ratelimit.by` | global default (ip) | Rate limit key | +| `simpledeploy.ratelimit.requests` | none (no limit) | Max requests per window. Required to turn rate limiting on | +| `simpledeploy.ratelimit.window` | `60s` | Time window | +| `simpledeploy.ratelimit.burst` | `0` | Extra requests allowed in a short spike, on top of `requests` | +| `simpledeploy.ratelimit.by` | `ip` | Rate limit key | ## Request Tracking Labels diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 707b03fb..0b75c89f 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -73,12 +73,11 @@ metrics: - name: 1h retention: 8760h # 1 year -# Global rate limit defaults (per-app labels override) +# Management API rate limit, per client IP. App traffic is limited only by +# per-app simpledeploy.ratelimit.* labels. ratelimit: requests: 200 window: 60s - burst: 50 - by: ip # ip | header:X-API-Key | path # Default registries applied to all apps (names reference stored registries) registries: @@ -121,10 +120,10 @@ git_sync: | `master_secret` | string | **required** | Encryption/signing key | | `audit_retention_days` | int | `365` | Activity log retention in days; `0` = keep forever | | `metrics.tiers` | list | see above | Metrics rollup tiers | -| `ratelimit.requests` | int | `200` | Default requests per window | -| `ratelimit.window` | string | `60s` | Rate limit time window | -| `ratelimit.burst` | int | `50` | Burst allowance | -| `ratelimit.by` | string | `ip` | Rate limit key | +| `ratelimit.requests` | int | `200` | Management API requests per window, per client IP | +| `ratelimit.window` | string | `60s` | Management API rate limit window | +| `ratelimit.burst` | int | `50` | Reserved, currently ignored. Per-app burst is the `simpledeploy.ratelimit.burst` label | +| `ratelimit.by` | string | `ip` | Reserved, currently ignored (the management API always limits by IP) | | `registries` | list | `[]` | Default registry names for all apps | | `allowed_bind_paths` | list of string | `[]` | Absolute host folders apps may bind-mount even though they sit under a protected system folder (e.g. `["/home/media"]`). SimpleDeploy's data folder and other apps' folders stay protected. Do not list parents of sensitive folders such as `/var` or `/etc`. See [compose security validation](/reference/compose-labels/#compose-security-validation). | | `git_sync.enabled` | bool | `false` | Enable two-way git sync | diff --git a/internal/proxy/ratelimit.go b/internal/proxy/ratelimit.go index fc1d2dbf..4534770a 100644 --- a/internal/proxy/ratelimit.go +++ b/internal/proxy/ratelimit.go @@ -128,6 +128,8 @@ type domainLimiter struct { type rlBucket struct { key string tokens int + spare int // burst allowance left, used once tokens run out + over bool // a request went past tokens this window lastReset time.Time lastAccess time.Time } @@ -171,28 +173,40 @@ func (d *domainLimiter) allow(key string) bool { } else { if len(d.buckets) >= d.maxBuckets { // Full. Buckets idle for a whole window would restart with full - // tokens anyway, so dropping them loses nothing; if none are, - // drop the least recently used. + // tokens anyway, so dropping them loses little (at most a spent + // burst); if none are, drop the least recently used. d.evictIdle(now.Add(-d.window)) for len(d.buckets) >= d.maxBuckets && d.lru.Len() > 0 { d.removeElement(d.lru.Back()) } } - b = &rlBucket{key: key, tokens: d.requests, lastReset: now} + b = &rlBucket{key: key, tokens: d.requests, spare: d.burst, lastReset: now} d.buckets[key] = d.lru.PushFront(b) } b.lastAccess = now - if now.Sub(b.lastReset) >= d.window { + if elapsed := now.Sub(b.lastReset); elapsed >= d.window { + // The burst comes back only after a whole window within requests + // (or with no requests at all), so a client that keeps going over + // is held to the steady rate. + if !b.over || elapsed >= 2*d.window { + b.spare = d.burst + } + b.over = false b.tokens = d.requests b.lastReset = now } - if b.tokens <= 0 { - return false + if b.tokens > 0 { + b.tokens-- + return true + } + b.over = true + if b.spare > 0 { + b.spare-- + return true } - b.tokens-- - return true + return false } // evictIdle removes buckets last used before cutoff, oldest first. diff --git a/internal/proxy/ratelimit_test.go b/internal/proxy/ratelimit_test.go index 550dadd5..af1bbd40 100644 --- a/internal/proxy/ratelimit_test.go +++ b/internal/proxy/ratelimit_test.go @@ -291,3 +291,55 @@ func TestRateLimitHandlerHostWithPortAndCase(t *testing.T) { } } } + +// allowN sends n requests for key and returns how many were allowed. +func allowN(d *domainLimiter, key string, n int) int { + ok := 0 + for i := 0; i < n; i++ { + if d.allow(key) { + ok++ + } + } + return ok +} + +func TestDomainLimiterBurst(t *testing.T) { + now := time.Unix(1000, 0) + d := newDomainLimiter(&RateLimitConfig{Requests: 3, Window: time.Minute, Burst: 2, By: "ip"}) + d.now = func() time.Time { return now } + + steps := []struct { + name string + send, got int + }{ + {"spike uses burst", 10, 5}, + {"burst spent: held to requests", 10, 3}, + {"still over: burst stays spent", 10, 3}, + {"calm window", 2, 2}, + {"burst back after a calm window", 10, 5}, + } + for _, s := range steps { + if got := allowN(d, "a", s.send); got != s.got { + t.Fatalf("%s: allowed %d of %d, want %d", s.name, got, s.send, s.got) + } + now = now.Add(time.Minute) + } + + // A whole window with no requests also brings the burst back. + now = now.Add(time.Minute) + if got := allowN(d, "a", 10); got != 5 { + t.Fatalf("after idle window: allowed %d, want 5", got) + } +} + +func TestDomainLimiterNoBurstIsFixedWindow(t *testing.T) { + now := time.Unix(1000, 0) + d := newDomainLimiter(&RateLimitConfig{Requests: 3, Window: time.Minute, By: "ip"}) + d.now = func() time.Time { return now } + for i := 0; i < 4; i++ { + if got := allowN(d, "a", 10); got != 3 { + t.Fatalf("window %d: allowed %d, want 3", i, got) + } + now = now.Add(time.Minute) + } +} From 0b5f53a8bb427ccff6364ee8836bd50534262f9e Mon Sep 17 00:00:00 2001 From: Ameen Rashad <7609895+vazra@users.noreply.github.com> Date: Sun, 11 Oct 2026 02:06:32 -0700 Subject: [PATCH 5/6] fix: mark activity rows sync-pending only while git sync pushes The recorder marked config changes pending even with git sync off, so Activity showed a pending badge forever. Recorder.SetSyncTracking takes api.Server.GitSyncPushing (kept in step with SetGitSync/ReloadGitSync); rows are pending only while a syncer with auto-push runs. --- cmd/simpledeploy/main.go | 2 + docs/concepts/activity-log.md | 2 +- docs/operations/security-audit.md | 2 +- internal/api/gitsync_config_handler.go | 15 ++++++ internal/api/gitsync_config_handler_test.go | 58 +++++++++++++++++++++ internal/api/server.go | 7 ++- internal/audit/recorder.go | 24 ++++++++- internal/audit/recorder_test.go | 41 +++++++++++++++ 8 files changed, 146 insertions(+), 5 deletions(-) diff --git a/cmd/simpledeploy/main.go b/cmd/simpledeploy/main.go index cf7a3b9d..2ae3f7fe 100644 --- a/cmd/simpledeploy/main.go +++ b/cmd/simpledeploy/main.go @@ -787,6 +787,8 @@ func runServe(cmd *cobra.Command, args []string) error { srv.SetWebhookDispatcher(dispatcher) auditRec := audit.NewRecorder(db) auditRec.SetBus(eventBus) + auditRec.SetSyncTracking(srv.GitSyncPushing) + reconcilerAudit.SetSyncTracking(srv.GitSyncPushing) srv.SetAudit(auditRec) srv.SetBus(eventBus) dep.SetAuditEmitter(&api.DeployerAuditAdapter{Rec: auditRec}) diff --git a/docs/concepts/activity-log.md b/docs/concepts/activity-log.md index cafb3555..ad7f4f5b 100644 --- a/docs/concepts/activity-log.md +++ b/docs/concepts/activity-log.md @@ -19,7 +19,7 @@ Each entry stores: actor, source (UI / API / CLI / git sync / system), category, ## Sync eligibility -Entries for config-change categories are marked with a sync status when [git sync](/operations/git-sync/) is enabled. The git sync worker stamps each relevant entry with `synced`, `pending`, or `failed` after the push completes. Runtime events (deploys, auth, start/stop) are not git-tracked and carry no badge. +Entries for config-change categories are marked with a sync status when [git sync](/operations/git-sync/) is enabled and pushes changes (auto-push on); entries recorded while it is off carry no badge. The git sync worker stamps each relevant entry with `synced`, `pending`, or `failed` after the push completes. Runtime events (deploys, auth, start/stop) are not git-tracked and carry no badge. ## Retention diff --git a/docs/operations/security-audit.md b/docs/operations/security-audit.md index f8a4424b..e86efa98 100644 --- a/docs/operations/security-audit.md +++ b/docs/operations/security-audit.md @@ -41,7 +41,7 @@ Entries for config-change categories (`compose`, `endpoint`, `backup`, `alert`, | Pending | Waiting for the next sync cycle | | Sync failed | Commit or push failed; see git sync error details | -Runtime-only events (`deploy`, `auth`, `system`, `lifecycle` start/stop/scale) are never committed to git and show no badge. +Runtime-only events (`deploy`, `auth`, `system`, `lifecycle` start/stop/scale) are never committed to git and show no badge. Entries recorded while git sync is off, or while auto-push is off (pull-only), also show no badge. ## Retention diff --git a/internal/api/gitsync_config_handler.go b/internal/api/gitsync_config_handler.go index 04597fba..6c952972 100644 --- a/internal/api/gitsync_config_handler.go +++ b/internal/api/gitsync_config_handler.go @@ -27,6 +27,19 @@ func (s *Server) SetReconcilerRef(r gitsync.Reconciler) { reconcilerRef = r } +// GitSyncPushing reports whether git sync is running and pushes changes. +// The audit recorder uses it to mark rows pending only when a push will +// stamp them. Lock-free, since audit rows may be recorded under gsMu. +func (s *Server) GitSyncPushing() bool { return s.gsPushing.Load() } + +func gitSyncPushes(gs *gitsync.Syncer) bool { + if gs == nil { + return false + } + st := gs.Status() + return st.Enabled && st.AutoPushEnabled +} + // ReloadGitSync stops the current syncer (if any), rebuilds config from DB+YAML, // and starts a new syncer. Returns the new status or an error. func (s *Server) ReloadGitSync(ctx context.Context) (*gitsync.Status, error) { @@ -39,6 +52,7 @@ func (s *Server) ReloadGitSync(ctx context.Context) (*gitsync.Status, error) { log.Printf("[gitsync] reload: stop existing: %v", err) } s.gs = nil + s.gsPushing.Store(false) } if s.cfg == nil || s.store == nil { @@ -71,6 +85,7 @@ func (s *Server) ReloadGitSync(ctx context.Context) (*gitsync.Status, error) { } s.gs = gs + s.gsPushing.Store(gitSyncPushes(gs)) // Re-wire configsync sidecar hook. if s.cs != nil { diff --git a/internal/api/gitsync_config_handler_test.go b/internal/api/gitsync_config_handler_test.go index 09f01a54..786e67b3 100644 --- a/internal/api/gitsync_config_handler_test.go +++ b/internal/api/gitsync_config_handler_test.go @@ -2,6 +2,7 @@ package api import ( "bytes" + "context" "encoding/json" "net/http" "net/http/httptest" @@ -12,6 +13,7 @@ import ( "github.com/vazra/simpledeploy/internal/auth" "github.com/vazra/simpledeploy/internal/config" + "github.com/vazra/simpledeploy/internal/gitsync" ) func gitPlainInit(path string, bare bool) (*git.Repository, error) { @@ -312,3 +314,59 @@ func TestDisableGitSync(t *testing.T) { t.Errorf("expected empty DB after disable, got %v", kv) } } + +func TestGitSyncPushingFollowsSyncer(t *testing.T) { + srv, _ := newTestServer(t) + newSyncer := func(push bool) *gitsync.Syncer { + t.Helper() + gs, err := gitsync.New(gitsync.Config{ + Enabled: true, + Remote: "file://" + t.TempDir(), + Branch: "main", + AppsDir: t.TempDir(), + AutoPushEnabled: push, + }, srv.store, nil, nil) + if err != nil { + t.Fatalf("gitsync.New: %v", err) + } + return gs + } + + if srv.GitSyncPushing() { + t.Error("no syncer: want false") + } + srv.SetGitSync(newSyncer(false)) + if srv.GitSyncPushing() { + t.Error("pull-only syncer: want false") + } + srv.SetGitSync(newSyncer(true)) + if !srv.GitSyncPushing() { + t.Error("pushing syncer: want true") + } + + // Reloading with git sync turned off clears it. + srv.cfg = &config.Config{AppsDir: t.TempDir(), MasterSecret: "secret"} + srv.masterSecret = "secret" + if _, err := srv.ReloadGitSync(context.Background()); err != nil { + t.Fatalf("ReloadGitSync: %v", err) + } + if srv.GitSyncPushing() { + t.Error("after reload with git sync off: want false") + } + + // Reloading with git sync on sets it again. + bare := t.TempDir() + if _, err := gitPlainInit(bare, true); err != nil { + t.Fatal(err) + } + if err := srv.store.SetGitSyncConfig(map[string]string{"enabled": "true", "remote": "file://" + bare, "branch": "main"}); err != nil { + t.Fatal(err) + } + if _, err := srv.ReloadGitSync(context.Background()); err != nil { + t.Fatalf("ReloadGitSync: %v", err) + } + t.Cleanup(func() { _ = srv.gs.Stop() }) + if !srv.GitSyncPushing() { + t.Error("after reload with git sync on: want true") + } +} diff --git a/internal/api/server.go b/internal/api/server.go index 87a805be..090c7d45 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -13,6 +13,7 @@ import ( "strconv" "strings" "sync" + "sync/atomic" "time" "github.com/robfig/cron/v3" @@ -79,6 +80,7 @@ type Server struct { cfgPath string deploymentMode deployment.Mode gs *gitsync.Syncer + gsPushing atomic.Bool // gs is set and pushes; see GitSyncPushing cs *configsync.Syncer recipesCache *recipes.Cache bus *events.Bus @@ -164,7 +166,10 @@ func (s *Server) SetConfig(cfg *config.Config, path string) { func (s *Server) SetDocker(dc docker.Client) { s.docker = dc } // SetGitSync sets the gitsync.Syncer (nil disables git endpoints). -func (s *Server) SetGitSync(gs *gitsync.Syncer) { s.gs = gs } +func (s *Server) SetGitSync(gs *gitsync.Syncer) { + s.gs = gs + s.gsPushing.Store(gitSyncPushes(gs)) +} // SetConfigSync sets the configsync.Syncer (nil disables sidecar cleanup on delete). func (s *Server) SetConfigSync(cs *configsync.Syncer) { s.cs = cs } diff --git a/internal/audit/recorder.go b/internal/audit/recorder.go index 2371aebb..77c7a26a 100644 --- a/internal/audit/recorder.go +++ b/internal/audit/recorder.go @@ -2,6 +2,7 @@ package audit import ( "context" + "sync/atomic" "github.com/vazra/simpledeploy/internal/audit/render" "github.com/vazra/simpledeploy/internal/events" @@ -17,8 +18,9 @@ type Publisher interface { // Recorder writes audit events to the DB-backed audit_log table. type Recorder struct { - s *store.Store - bus Publisher + s *store.Store + bus Publisher + syncOn atomic.Pointer[func() bool] } // NewRecorder creates a Recorder backed by the given store. @@ -33,6 +35,21 @@ func (r *Recorder) SetBus(b Publisher) { r.bus = b } +// SetSyncTracking sets how the recorder learns whether git sync is pushing +// changes. Sync-eligible rows are marked pending only while on returns true, +// so Activity does not show "pending" forever when git sync is off. nil (the +// default) marks every eligible row pending. Safe to call while recording. +func (r *Recorder) SetSyncTracking(on func() bool) { + if r == nil { + return + } + if on == nil { + r.syncOn.Store(nil) + return + } + r.syncOn.Store(&on) +} + // RecordReq is the input to Recorder.Record. type RecordReq struct { AppID *int64 @@ -73,6 +90,9 @@ func (r *Recorder) Record(ctx context.Context, req RecordReq) (int64, error) { if req.Category == "lifecycle" && syncEligibleLifecycleActions[req.Action] { eligible = true } + if on := r.syncOn.Load(); eligible && on != nil && !(*on)() { + eligible = false + } id, err := r.s.RecordAudit(ctx, store.AuditEntry{ AppID: req.AppID, AppSlug: req.AppSlug, diff --git a/internal/audit/recorder_test.go b/internal/audit/recorder_test.go index 656c27f5..1f55e62d 100644 --- a/internal/audit/recorder_test.go +++ b/internal/audit/recorder_test.go @@ -134,3 +134,44 @@ func TestRecorderNilSafe(t *testing.T) { t.Fatalf("nil Recorder.Record returned non-zero id: %d", id) } } + +func TestRecorderSyncPendingOnlyWhileGitSyncPushes(t *testing.T) { + s := openTestStore(t) + rec := NewRecorder(s) + ctx := context.Background() + status := func(id int64) string { + t.Helper() + got, err := s.GetActivity(ctx, id) + if err != nil { + t.Fatalf("GetActivity: %v", err) + } + if got.SyncStatus == nil { + return "" + } + return *got.SyncStatus + } + + pushing := false + rec.SetSyncTracking(func() bool { return pushing }) + + id, _ := rec.Record(ctx, RecordReq{Category: "compose", Action: "changed"}) + if got := status(id); got != "" { + t.Errorf("git sync off: sync_status = %q, want none", got) + } + + pushing = true + id, _ = rec.Record(ctx, RecordReq{Category: "compose", Action: "changed"}) + if got := status(id); got != "pending" { + t.Errorf("git sync on: sync_status = %q, want pending", got) + } + id, _ = rec.Record(ctx, RecordReq{Category: "lifecycle", Action: "stopped"}) + if got := status(id); got != "" { + t.Errorf("non-synced action: sync_status = %q, want none", got) + } + + // The pending row is what git sync stamps after a push. + ids, err := s.PendingSyncAuditIDs(ctx) + if err != nil || len(ids) != 1 { + t.Fatalf("PendingSyncAuditIDs = %v, %v; want one row", ids, err) + } +} From 9a63cedb3fe4211b27157a27ce8ca40dfa8c44f8 Mon Sep 17 00:00:00 2001 From: Ameen Rashad <7609895+vazra@users.noreply.github.com> Date: Sun, 11 Oct 2026 02:08:18 -0700 Subject: [PATCH 6/6] fix(api): keep git sync running after saving its settings ReloadGitSync started the new syncer with the request context, so its worker and poll loop stopped as soon as the settings response was sent. --- internal/api/gitsync_config_handler.go | 3 +- internal/api/gitsync_config_handler_test.go | 42 +++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/internal/api/gitsync_config_handler.go b/internal/api/gitsync_config_handler.go index 6c952972..1d3961ec 100644 --- a/internal/api/gitsync_config_handler.go +++ b/internal/api/gitsync_config_handler.go @@ -80,7 +80,8 @@ func (s *Server) ReloadGitSync(ctx context.Context) (*gitsync.Status, error) { if err != nil { return nil, fmt.Errorf("gitsync.New: %w", err) } - if err := gs.Start(ctx); err != nil { + // ctx is usually the request's; the syncer must outlive it (Stop ends it). + if err := gs.Start(context.WithoutCancel(ctx)); err != nil { return nil, fmt.Errorf("gitsync.Start: %w", err) } diff --git a/internal/api/gitsync_config_handler_test.go b/internal/api/gitsync_config_handler_test.go index 786e67b3..bf927d66 100644 --- a/internal/api/gitsync_config_handler_test.go +++ b/internal/api/gitsync_config_handler_test.go @@ -6,6 +6,8 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "os" + "path/filepath" "testing" "time" @@ -370,3 +372,43 @@ func TestGitSyncPushingFollowsSyncer(t *testing.T) { t.Error("after reload with git sync on: want true") } } + +// The handlers reload with r.Context(), which ends with the response. The +// syncer started by the reload must keep running after that. +func TestReloadGitSyncOutlivesRequest(t *testing.T) { + srv, _ := newTestServer(t) + appsDir := t.TempDir() + srv.cfg = &config.Config{AppsDir: appsDir, MasterSecret: "secret"} + srv.masterSecret = "secret" + bare := t.TempDir() + if _, err := gitPlainInit(bare, true); err != nil { + t.Fatal(err) + } + if err := srv.store.SetGitSyncConfig(map[string]string{"enabled": "true", "remote": "file://" + bare, "branch": "main"}); err != nil { + t.Fatal(err) + } + + reqCtx, endRequest := context.WithCancel(context.Background()) + if _, err := srv.ReloadGitSync(reqCtx); err != nil { + t.Fatalf("ReloadGitSync: %v", err) + } + t.Cleanup(func() { _ = srv.gs.Stop() }) + endRequest() + time.Sleep(100 * time.Millisecond) // let a request-bound worker exit + + before := srv.gs.Status().HeadSHA + if err := os.MkdirAll(filepath.Join(appsDir, "foo"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(appsDir, "foo", "docker-compose.yml"), []byte("services: {}\n"), 0o644); err != nil { + t.Fatal(err) + } + srv.gs.EnqueueCommit(nil, "test edit") + deadline := time.Now().Add(10 * time.Second) + for srv.gs.Status().HeadSHA == before { + if time.Now().After(deadline) { + t.Fatal("syncer stopped committing once the reload request ended") + } + time.Sleep(50 * time.Millisecond) + } +}