diff --git a/.env.template b/.env.template index 5d54f2591..7817c3a03 100644 --- a/.env.template +++ b/.env.template @@ -4,11 +4,11 @@ # `mise run dev:server` load .env via the [env] block in mise.toml, so the # values flow into the server process automatically. # -# Required only when running in "public mode" — i.e. when $PORT or -# $STACKIT_PUBLIC is set (Railway, Fly, Heroku, etc). For purely local -# development (`mise run dev`), every STACKIT_GITHUB_* / STACKIT_SESSION_KEY -# / STACKIT_ALLOWED_GH_* variable can stay blank and the server will start -# with auth disabled. +# Required only in production (STACKIT_ENV=production, e.g. Railway, Fly, +# Heroku), which binds 0.0.0.0 and requires auth (or -read-only). For purely +# local development (`mise run dev`, the default STACKIT_ENV=local), every +# STACKIT_GITHUB_* / STACKIT_SESSION_KEY / STACKIT_ALLOWED_GH_* variable can +# stay blank and the server binds loopback with auth disabled. # --- GitHub OAuth app (https://github.com/settings/developers) ---------------- # Create an OAuth App with: @@ -37,12 +37,14 @@ STACKIT_ALLOWED_GH_ORG= # Required when -repos-config entries use owner+name instead of an absolute path. STACKIT_REPOS_ROOT= -# --- Public mode override (rare) ---------------------------------------------- -# Set non-empty to force public-mode behavior (binds 0.0.0.0, requires auth) -# even without $PORT. Leave blank for local dev. -# STACKIT_PUBLIC= +# --- Deployment posture ------------------------------------------------------- +# `local` (default) binds 127.0.0.1 with auth optional — leave it unset for dev. +# `production` binds 0.0.0.0, emits JSON logs, forces Secure cookies, honors +# $PORT, and requires auth (or -read-only). Set it on hosted deploys. +# STACKIT_ENV=production # --- Port override ------------------------------------------------------------ -# PaaS hosts inject $PORT automatically; set it here only if running the -# binary directly without `mise run dev`. +# Only honored in production (PaaS hosts inject $PORT automatically). Ignored +# in local, so a stray $PORT in your shell can't move the dev listener; pass +# `-port` to the binary instead. # PORT=8080 diff --git a/api/openapi/stackit.yaml b/api/openapi/stackit.yaml index bd7b9480e..8bed904e8 100644 --- a/api/openapi/stackit.yaml +++ b/api/openapi/stackit.yaml @@ -6,7 +6,54 @@ info: servers: - url: / paths: - /api/v1/view: + /api/v1/repos: + get: + summary: List repositories the server is serving (filtered to the caller). + operationId: listRepos + responses: + "200": + description: Repository index. + content: + application/json: + schema: + $ref: "#/components/schemas/ReposListResponse" + post: + summary: Onboard a GitHub repository (clone and start serving it). + description:> + Clones a GitHub repository the authenticated user can access and starts + serving it, recorded against that user so only they see it. Requires an + authenticated, writable, DB-backed server with a repos root configured; + read-only servers refuse with 405. + operationId: onboardRepo + requestBody: + required: true + content: + application/json: + schema: + $ref: "#/components/schemas/OnboardRepoRequest" + responses: + "201": + description: The newly served repository. + content: + application/json: + schema: + $ref: "#/components/schemas/RepoSummary" + "400": + description: Missing or invalid owner/name. + "401": + description: Authentication required. + "404": + description: Repository not found or not accessible to the caller. + "405": + description: Server is read-only. + "409": + description: Repository already onboarded. + "503": + description: Onboarding not available (no auth, database, or repos root). + /api/v1/repos/{owner}/{repo}/view: + parameters: + - $ref: "#/components/parameters/Owner" + - $ref: "#/components/parameters/Repo" get: summary: Combined view payload for the dashboard. operationId: getView @@ -17,7 +64,12 @@ paths: application/json: schema: $ref: "#/components/schemas/ViewResponse" - /api/v1/repo: + "404": + description: Repository not found or not visible to the caller. + /api/v1/repos/{owner}/{repo}/repo: + parameters: + - $ref: "#/components/parameters/Owner" + - $ref: "#/components/parameters/Repo" get: summary: Repository metadata. operationId: getRepo @@ -28,7 +80,12 @@ paths: application/json: schema: $ref: "#/components/schemas/RepoResponse" - /api/v1/stacks: + "404": + description: Repository not found or not visible to the caller. + /api/v1/repos/{owner}/{repo}/stacks: + parameters: + - $ref: "#/components/parameters/Owner" + - $ref: "#/components/parameters/Repo" get: summary: List discovered stacks. operationId: listStacks @@ -41,7 +98,10 @@ paths: type: array items: $ref: "#/components/schemas/StackSummary" - /api/v1/stacks/{rootBranch}: + /api/v1/repos/{owner}/{repo}/stacks/{rootBranch}: + parameters: + - $ref: "#/components/parameters/Owner" + - $ref: "#/components/parameters/Repo" get: summary: Get detailed stack by root branch. operationId: getStack @@ -60,7 +120,46 @@ paths: $ref: "#/components/schemas/StackDetail" "404": description: Stack not found. - /api/v1/branches: + /api/v1/repos/{owner}/{repo}/stacks/{rootBranch}/submit: + parameters: + - $ref: "#/components/parameters/Owner" + - $ref: "#/components/parameters/Repo" + post: + summary: Submit a stack as stacked pull requests. + operationId: submitStack + parameters: + - in: path + name: rootBranch + required: true + schema: + type: string + responses: + "200": + description: Submit result. + "404": + description: Stack not found. + "405": + description: Server is read-only. + /api/v1/repos/{owner}/{repo}/sync: + parameters: + - $ref: "#/components/parameters/Owner" + - $ref: "#/components/parameters/Repo" + post: + summary: Force an immediate refresh of one repository. + operationId: syncRepo + responses: + "204": + description: Refresh completed. + "404": + description: Repository not found or not visible to the caller. + "405": + description: Server is read-only. + "502": + description: Mirror fetch failed. + /api/v1/repos/{owner}/{repo}/branches: + parameters: + - $ref: "#/components/parameters/Owner" + - $ref: "#/components/parameters/Repo" get: summary: List tracked branches. operationId: listBranches @@ -73,7 +172,10 @@ paths: type: array items: $ref: "#/components/schemas/BranchResponse" - /api/v1/branches/{branchName}: + /api/v1/repos/{owner}/{repo}/branches/{branchName}: + parameters: + - $ref: "#/components/parameters/Owner" + - $ref: "#/components/parameters/Repo" get: summary: Get one tracked branch. operationId: getBranch @@ -92,13 +194,16 @@ paths: $ref: "#/components/schemas/BranchResponse" "404": description: Branch not found. - /api/v1/branch-diff: + /api/v1/repos/{owner}/{repo}/branch-diff/{branchName}: + parameters: + - $ref: "#/components/parameters/Owner" + - $ref: "#/components/parameters/Repo" get: summary: Get raw patch diff for a tracked branch. operationId: getBranchDiff parameters: - - in: query - name: branch + - in: path + name: branchName required: true schema: type: string @@ -110,10 +215,13 @@ paths: schema: $ref: "#/components/schemas/BranchDiffResponse" "400": - description: Missing branch query parameter. + description: Missing branch. "404": description: Branch not found. - /api/v1/events: + /api/v1/repos/{owner}/{repo}/events: + parameters: + - $ref: "#/components/parameters/Owner" + - $ref: "#/components/parameters/Repo" get: summary: Server-sent event stream for repository refresh events. operationId: streamEvents @@ -124,51 +232,66 @@ paths: text/event-stream: schema: type: string - /api/v1/repos: - get: - summary: List repositories the server is serving (filtered to the caller). - operationId: listRepos - responses: - "200": - description: Repository index. - content: - application/json: - schema: - $ref: "#/components/schemas/ReposListResponse" + /api/v1/webhooks/github: post: - summary: Onboard a GitHub repository (clone and start serving it). - description:> - Clones a GitHub repository the authenticated user can access and starts - serving it, recorded against that user so only they see it. Requires an - authenticated, writable, DB-backed server with a repos root configured; - read-only servers refuse with 405. - operationId: onboardRepo + summary: Receive a GitHub webhook delivery and trigger a repo refresh. + description:>- + Authenticated solely by the X-Hub-Signature-256 HMAC over the raw body; + it carries no session or CSRF token. Fails closed: when no webhook secret + is configured the endpoint returns 404. A verified push is acked + immediately and the matching managed repo is mirror-fetched off the + request path (a burst for one repo coalesces into a single fetch). It is + unaffected by read-only mode, since a refresh is a read-side operation. + operationId: receiveGitHubWebhook + parameters: + - in: header + name: X-Hub-Signature-256 + required: true + description: HMAC-SHA256 of the raw request body, keyed by the webhook secret. + schema: + type: string + - in: header + name: X-GitHub-Event + required: true + description: GitHub event name; only "push" triggers a refresh and "ping" is acked. + schema: + type: string requestBody: required: true content: application/json: schema: - $ref: "#/components/schemas/OnboardRepoRequest" + type: object + description: The GitHub webhook event payload (e.g. a push event). responses: - "201": - description: The newly served repository. - content: - application/json: - schema: - $ref: "#/components/schemas/RepoSummary" + "202": + description: Push accepted; a refresh is scheduled for the matching managed repo. + "204": + description:>- + Verified but no action taken — a ping, a non-push event, or a push + that did not resolve to a managed repo. "400": - description: Missing or invalid owner/name. + description: Could not read the request body. "401": - description: Authentication required. + description: Missing or invalid signature. "404": - description: Repository not found or not accessible to the caller. - "405": - description: Server is read-only. - "409": - description: Repository already onboarded. - "503": - description: Onboarding not available (no auth, database, or repos root). + description: Webhook receiver is disabled (no secret configured). components: + parameters: + Owner: + in: path + name: owner + required: true + description: GitHub repository owner (login), matched case-insensitively. + schema: + type: string + Repo: + in: path + name: repo + required: true + description: GitHub repository name, matched case-insensitively. + schema: + type: string schemas: ViewResponse: type: object @@ -373,6 +496,10 @@ components: properties: id: type: string + owner: + type: string + repo: + type: string displayName: type: string trunk: diff --git a/apps/server/auth.go b/apps/server/auth.go index fd3b66cf6..576af25dc 100644 --- a/apps/server/auth.go +++ b/apps/server/auth.go @@ -16,14 +16,29 @@ type authBuildResult struct { store auth.Store // exposed separately so main can close it on shutdown } +// authBuildParams describes the runtime posture buildAuthConfig needs. The +// auth requirement keys off exposure (a non-loopback bind), not the env name: +// a server reachable off-host must be authenticated or read-only, however it +// got that way. +type authBuildParams struct { + disabled bool // -auth-disabled was passed + exposed bool // resolved bind is non-loopback (reachable off-host) + readOnly bool // read-only posture: writes impossible, reads anonymous + prod bool // production env: force Secure cookies (behind TLS) +} + // buildAuthConfig wires up the OAuth handler, session store, and allowlist -// from environment variables. Returns (nil, nil, nil) when auth is -// explicitly disabled; an error when required env is missing in public -// mode. -func buildAuthConfig(authDisabled, publicMode bool) (*authBuildResult, error) { - if authDisabled { - if publicMode { - return nil, errors.New("-auth-disabled is not allowed when $PORT or $STACKIT_PUBLIC are set; remove the flag or unset the env vars") +// from environment variables. Returns (nil, nil) when auth is off (explicitly +// disabled, or unconfigured on a non-exposed/read-only server); an error when +// an exposed, writable server would be left unauthenticated. +func buildAuthConfig(p authBuildParams) (*authBuildResult, error) { + // An exposed, writable server must be authenticated. Read-only removes the + // write route by construction, so anonymous reads are safe there. + mustAuth := p.exposed && !p.readOnly + + if p.disabled { + if mustAuth { + return nil, errors.New("-auth-disabled is not allowed when the server is reachable off-host (non-loopback bind): it would expose an unauthenticated, writable server. Pass -read-only, bind loopback (STACKIT_ENV=local), or configure GitHub OAuth") } return nil, nil } @@ -33,13 +48,12 @@ func buildAuthConfig(authDisabled, publicMode bool) (*authBuildResult, error) { baseURL := strings.TrimSpace(os.Getenv("STACKIT_BASE_URL")) sessionKey := strings.TrimSpace(os.Getenv("STACKIT_SESSION_KEY")) - // Outside public mode, missing OAuth config is "off by default" — - // users running a local dev binary shouldn't have to set env vars to - // boot. They can pass -auth-disabled explicitly to make the intent - // clear, or just leave the OAuth env unset. + // When the server isn't exposed (local loopback) or is read-only, missing + // OAuth config is "off by default" — a local dev binary shouldn't need env + // vars to boot, and a read-only server serves reads anonymously anyway. if clientID == "" && clientSecret == "" && baseURL == "" && sessionKey == "" { - if publicMode { - return nil, errors.New("public mode requires STACKIT_GITHUB_CLIENT_ID, STACKIT_GITHUB_CLIENT_SECRET, STACKIT_BASE_URL, STACKIT_SESSION_KEY (and STACKIT_ALLOWED_GH_USERS or STACKIT_ALLOWED_GH_ORG); pass -auth-disabled if you really mean to run open") + if mustAuth { + return nil, errors.New("the server is reachable off-host (non-loopback bind) but auth is not configured: set STACKIT_GITHUB_CLIENT_ID, STACKIT_GITHUB_CLIENT_SECRET, STACKIT_BASE_URL, STACKIT_SESSION_KEY (and STACKIT_ALLOWED_GH_USERS or STACKIT_ALLOWED_GH_ORG), or pass -read-only to serve anonymously") } return nil, nil } @@ -76,7 +90,7 @@ func buildAuthConfig(authDisabled, publicMode bool) (*authBuildResult, error) { ClientID: clientID, ClientSecret: clientSecret, BaseURL: baseURL, - Cookies: auth.CookieOptions{Secure: publicMode || strings.HasPrefix(baseURL, "https://")}, + Cookies: auth.CookieOptions{Secure: p.prod || strings.HasPrefix(baseURL, "https://")}, }, store, allow, cipher) if err != nil { _ = store.Close() diff --git a/apps/server/auth_test.go b/apps/server/auth_test.go index 71af52c94..8f90d9145 100644 --- a/apps/server/auth_test.go +++ b/apps/server/auth_test.go @@ -11,18 +11,28 @@ import ( func TestBuildAuthConfig_DisabledLocalOK(t *testing.T) { t.Parallel() - r, err := buildAuthConfig(true, false) + r, err := buildAuthConfig(authBuildParams{disabled: true}) require.NoError(t, err) require.Nil(t, r) } -func TestBuildAuthConfig_DisabledInPublicModeRefused(t *testing.T) { +func TestBuildAuthConfig_DisabledExposedRefused(t *testing.T) { t.Parallel() - _, err := buildAuthConfig(true, true) + _, err := buildAuthConfig(authBuildParams{disabled: true, exposed: true}) require.Error(t, err) } +func TestBuildAuthConfig_DisabledExposedReadOnlyOK(t *testing.T) { + t.Parallel() + + // Read-only removes the write route, so disabling auth on an exposed + // read-only server is safe — reads are anonymous by design. + r, err := buildAuthConfig(authBuildParams{disabled: true, exposed: true, readOnly: true}) + require.NoError(t, err) + require.Nil(t, r) +} + func TestBuildAuthConfig_NoEnvLocalFallsThroughToUnauthed(t *testing.T) { // t.Setenv is incompatible with t.Parallel; these tests share process env. withEnv(t, map[string]string{ @@ -34,12 +44,12 @@ func TestBuildAuthConfig_NoEnvLocalFallsThroughToUnauthed(t *testing.T) { "STACKIT_ALLOWED_GH_ORG": "", }) - r, err := buildAuthConfig(false, false) + r, err := buildAuthConfig(authBuildParams{}) require.NoError(t, err) - require.Nil(t, r, "no env + local mode = no auth, no error") + require.Nil(t, r, "no env + not exposed = no auth, no error") } -func TestBuildAuthConfig_NoEnvInPublicModeRefused(t *testing.T) { +func TestBuildAuthConfig_NoEnvExposedRefused(t *testing.T) { // t.Setenv is incompatible with t.Parallel; these tests share process env. withEnv(t, map[string]string{ "STACKIT_GITHUB_CLIENT_ID": "", @@ -50,10 +60,27 @@ func TestBuildAuthConfig_NoEnvInPublicModeRefused(t *testing.T) { "STACKIT_ALLOWED_GH_ORG": "", }) - _, err := buildAuthConfig(false, true) + _, err := buildAuthConfig(authBuildParams{exposed: true}) require.Error(t, err) } +func TestBuildAuthConfig_NoEnvExposedReadOnlyOK(t *testing.T) { + // t.Setenv is incompatible with t.Parallel; these tests share process env. + withEnv(t, map[string]string{ + "STACKIT_GITHUB_CLIENT_ID": "", + "STACKIT_GITHUB_CLIENT_SECRET": "", + "STACKIT_BASE_URL": "", + "STACKIT_SESSION_KEY": "", + "STACKIT_ALLOWED_GH_USERS": "", + "STACKIT_ALLOWED_GH_ORG": "", + }) + + // An exposed read-only server boots anonymously with no OAuth env. + r, err := buildAuthConfig(authBuildParams{exposed: true, readOnly: true}) + require.NoError(t, err) + require.Nil(t, r) +} + func TestBuildAuthConfig_PartialEnvErrors(t *testing.T) { // t.Setenv is incompatible with t.Parallel; these tests share process env. withEnv(t, map[string]string{ @@ -64,7 +91,7 @@ func TestBuildAuthConfig_PartialEnvErrors(t *testing.T) { "STACKIT_ALLOWED_GH_USERS": "jonnii", }) - _, err := buildAuthConfig(false, false) + _, err := buildAuthConfig(authBuildParams{}) require.Error(t, err) require.Contains(t, err.Error(), "STACKIT_GITHUB_CLIENT_SECRET") } @@ -79,7 +106,7 @@ func TestBuildAuthConfig_HappyPath(t *testing.T) { "STACKIT_ALLOWED_GH_USERS": "jonnii", }) - r, err := buildAuthConfig(false, false) + r, err := buildAuthConfig(authBuildParams{}) require.NoError(t, err) require.NotNil(t, r) require.NotNil(t, r.cfg.Handler) @@ -98,7 +125,7 @@ func TestBuildAuthConfig_EmptyAllowlistErrors(t *testing.T) { "STACKIT_ALLOWED_GH_ORG": "", }) - _, err := buildAuthConfig(false, false) + _, err := buildAuthConfig(authBuildParams{}) require.Error(t, err) require.Contains(t, err.Error(), "allowlist") } diff --git a/apps/server/bind.go b/apps/server/bind.go index 72d689007..82feb8d00 100644 --- a/apps/server/bind.go +++ b/apps/server/bind.go @@ -1,17 +1,22 @@ package main +const ( + bindLoopback = "127.0.0.1" + bindAllInterfaces = "0.0.0.0" +) + // resolveBind picks the default bind address when the operator did not pass // -bind explicitly. Locally we want 127.0.0.1 so the API isn't reachable to // other users on the host; PaaS / container deploys flip to 0.0.0.0 because -// the platform's router is on a different interface. publicHint is true when -// $PORT or $STACKIT_PUBLIC are set, both of which signal a hosted runtime. -func resolveBind(flagBind *string, bindExplicit, publicHint bool) { +// the platform's router is on a different interface. prod is true when +// STACKIT_ENV=production, which signals a hosted runtime. +func resolveBind(flagBind *string, bindExplicit, prod bool) { if bindExplicit { return } - if publicHint { - *flagBind = "0.0.0.0" + if prod { + *flagBind = bindAllInterfaces return } - *flagBind = "127.0.0.1" + *flagBind = bindLoopback } diff --git a/apps/server/bind_test.go b/apps/server/bind_test.go index d4b4453a9..6776204f5 100644 --- a/apps/server/bind_test.go +++ b/apps/server/bind_test.go @@ -6,25 +6,25 @@ func TestResolveBind(t *testing.T) { t.Parallel() cases := []struct { - name string - bind string - explicit bool - publicHint bool - want string + name string + bind string + explicit bool + prod bool + want string }{ {name: "explicit operator value wins", bind: "10.0.0.5", explicit: true, want: "10.0.0.5"}, {name: "explicit empty value wins too", bind: "", explicit: true, want: ""}, - {name: "PaaS hint flips to all interfaces", publicHint: true, want: "0.0.0.0"}, + {name: "production flips to all interfaces", prod: true, want: "0.0.0.0"}, {name: "local default is loopback", want: "127.0.0.1"}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { t.Parallel() b := tc.bind - resolveBind(&b, tc.explicit, tc.publicHint) + resolveBind(&b, tc.explicit, tc.prod) if b != tc.want { - t.Fatalf("resolveBind(%q, explicit=%v, publicHint=%v) = %q, want %q", - tc.bind, tc.explicit, tc.publicHint, b, tc.want) + t.Fatalf("resolveBind(%q, explicit=%v, prod=%v) = %q, want %q", + tc.bind, tc.explicit, tc.prod, b, tc.want) } }) } diff --git a/apps/server/env.go b/apps/server/env.go new file mode 100644 index 000000000..d8a3b26f1 --- /dev/null +++ b/apps/server/env.go @@ -0,0 +1,62 @@ +package main + +import ( + "fmt" + "net" + "strings" +) + +// serverEnv selects the deployment posture: a coherent bundle of defaults for +// the bind interface, log format, cookie security, and whether $PORT is +// honored. It is the single source of truth for "am I running locally or +// hosted", replacing the old heuristic that inferred this from $PORT — which +// collided with dev shells (cmux, Heroku-style toolchains) that export $PORT +// generically and silently flipped a local server into a public, auth-gated one. +type serverEnv int + +const ( + envLocal serverEnv = iota + envProduction +) + +const ( + envNameLocal = "local" + envNameProduction = "production" +) + +func (e serverEnv) isProduction() bool { return e == envProduction } + +func (e serverEnv) String() string { + if e == envProduction { + return envNameProduction + } + return envNameLocal +} + +// resolveEnv parses STACKIT_ENV. Unset (or "local"/"development") means local; +// "production" means hosted. Unrecognized values are a hard error so a typo on +// a deploy fails loudly rather than silently picking the wrong posture. The +// default is local — the fail-safe direction, since local binds loopback: a +// hosted deploy that forgets the var becomes unreachable, never exposed. +func resolveEnv(raw string) (serverEnv, error) { + switch strings.ToLower(strings.TrimSpace(raw)) { + case "", envNameLocal, "development", "dev": + return envLocal, nil + case envNameProduction, "prod": + return envProduction, nil + default: + return envLocal, fmt.Errorf("invalid STACKIT_ENV %q: want \"local\" or \"production\"", raw) + } +} + +// isLoopbackBind reports whether addr binds only the loopback interface, so an +// unauthenticated server on it is not reachable off-host. Empty ("all +// interfaces"), 0.0.0.0, and :: are treated as exposed, not loopback. +func isLoopbackBind(addr string) bool { + addr = strings.TrimSpace(addr) + if addr == "localhost" { + return true + } + ip := net.ParseIP(addr) + return ip != nil && ip.IsLoopback() +} diff --git a/apps/server/env_test.go b/apps/server/env_test.go new file mode 100644 index 000000000..a6c5078d5 --- /dev/null +++ b/apps/server/env_test.go @@ -0,0 +1,75 @@ +package main + +import "testing" + +func TestResolveEnv(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + raw string + want serverEnv + wantErr bool + }{ + {name: "unset defaults to local", raw: "", want: envLocal}, + {name: "local", raw: "local", want: envLocal}, + {name: "local uppercase", raw: "LOCAL", want: envLocal}, + {name: "development alias", raw: "development", want: envLocal}, + {name: "dev alias", raw: "dev", want: envLocal}, + {name: "production", raw: "production", want: envProduction}, + {name: "production trimmed and cased", raw: " Production ", want: envProduction}, + {name: "prod alias", raw: "prod", want: envProduction}, + {name: "unknown is an error", raw: "staging", wantErr: true}, + {name: "typo is an error", raw: "prdouction", wantErr: true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got, err := resolveEnv(tc.raw) + if tc.wantErr { + if err == nil { + t.Fatalf("resolveEnv(%q) = %v, want error", tc.raw, got) + } + // Even on error, the fail-safe fallback is local. + if got != envLocal { + t.Fatalf("resolveEnv(%q) error fallback = %v, want envLocal", tc.raw, got) + } + return + } + if err != nil { + t.Fatalf("resolveEnv(%q) unexpected error: %v", tc.raw, err) + } + if got != tc.want { + t.Fatalf("resolveEnv(%q) = %v, want %v", tc.raw, got, tc.want) + } + }) + } +} + +func TestIsLoopbackBind(t *testing.T) { + t.Parallel() + + cases := []struct { + addr string + want bool + }{ + {addr: "127.0.0.1", want: true}, + {addr: "127.0.0.2", want: true}, // 127.0.0.0/8 is all loopback + {addr: "::1", want: true}, + {addr: "localhost", want: true}, + {addr: " 127.0.0.1 ", want: true}, // trimmed + {addr: "", want: false}, // empty = all interfaces = exposed + {addr: "0.0.0.0", want: false}, + {addr: "::", want: false}, + {addr: "192.168.1.5", want: false}, + {addr: "10.0.0.5", want: false}, + } + for _, tc := range cases { + t.Run(tc.addr, func(t *testing.T) { + t.Parallel() + if got := isLoopbackBind(tc.addr); got != tc.want { + t.Fatalf("isLoopbackBind(%q) = %v, want %v", tc.addr, got, tc.want) + } + }) + } +} diff --git a/apps/server/main.go b/apps/server/main.go index e103ed5a3..34de68acf 100644 --- a/apps/server/main.go +++ b/apps/server/main.go @@ -45,7 +45,7 @@ func main() { } // setupLogging configures the default slog logger. Production deploys -// (PORT or STACKIT_PUBLIC set) emit JSON with a level field so log +// (STACKIT_ENV=production) emit JSON with a level field so log // aggregators tag entries correctly; local runs use the text handler // for readability. Output goes to stdout so platforms like Railway // don't classify everything as error (which is what happens when the @@ -66,7 +66,7 @@ func setupLogging(jsonOutput bool) { func run() error { var ( port = flag.Int("port", 8080, "Port to listen on") - bind = flag.String("bind", "", "Interface to bind on. Defaults to 127.0.0.1; switches to 0.0.0.0 when $PORT or $STACKIT_PUBLIC is set.") + bind = flag.String("bind", "", "Interface to bind on. Defaults to 127.0.0.1 (loopback); STACKIT_ENV=production binds 0.0.0.0.") cwd = flag.String("cwd", "", "Working directory for repository detection (single-repo shortcut; ignored when -database-url is set)") databaseURL = flag.String("database-url", os.Getenv("STACKIT_DATABASE_URL"), "PostgreSQL connection string. When set, repos are served from the DB instead of the -cwd single-repo shortcut.") reposRoot = flag.String("repos-root", os.Getenv("STACKIT_REPOS_ROOT"), "Base directory under which per-repo checkouts live (//) for DB-sourced repos.") @@ -75,19 +75,21 @@ func run() error { apiPrefix = flag.String("api-prefix", "/api/v1", "Canonical API prefix") enableLegacy = flag.Bool("legacy-api-prefix", true, "Also expose legacy /api endpoints") shutdownGrace = flag.Duration("shutdown-timeout", 10*time.Second, "Graceful shutdown timeout") - authDisabled = flag.Bool("auth-disabled", false, "Disable GitHub OAuth gate. Refused in public mode ($PORT or $STACKIT_PUBLIC).") + authDisabled = flag.Bool("auth-disabled", false, "Disable GitHub OAuth gate. Refused when the server binds a non-loopback interface (e.g. STACKIT_ENV=production) unless -read-only is set.") readOnly = flag.Bool("read-only", envBool("STACKIT_READ_ONLY"), "Serve in read-only mode: the submit endpoint is disabled so the repo can be exposed publicly without write access. Also set via STACKIT_READ_ONLY.") - syncInterval = flag.Duration("sync-interval", envDuration("STACKIT_SYNC_INTERVAL"), "How often to mirror-fetch managed repos from their remotes so served state stays current. 0 disables the loop. Also set via STACKIT_SYNC_INTERVAL (e.g. 60s).") + syncInterval = flag.Duration("sync-interval", envSyncInterval(), "How often to mirror-fetch managed repos from their remotes so served state stays current. Defaults to 5m (the webhook backstop); 0 disables the loop. Also set via STACKIT_SYNC_INTERVAL (e.g. 60s).") ) flag.Parse() - publicMode := os.Getenv("PORT") != "" || os.Getenv("STACKIT_PUBLIC") != "" - setupLogging(publicMode) + env, err := resolveEnv(os.Getenv("STACKIT_ENV")) + if err != nil { + return err + } + prod := env.isProduction() + setupLogging(prod) - slog.Info("stackit-server starting", "version", version, "commit", commit, "built", date) + slog.Info("stackit-server starting", "version", version, "commit", commit, "built", date, "env", env.String()) - // Honor $PORT when -port wasn't passed explicitly. PaaS hosts (Railway, - // Fly, Heroku) inject the port this way. portExplicit := false bindExplicit := false cwdExplicit := false @@ -101,10 +103,18 @@ func run() error { cwdExplicit = true } }) - if err := resolvePort(port, portExplicit, os.Getenv("PORT")); err != nil { + // $PORT is the PaaS convention (Railway, Fly, Heroku inject it); honored + // only in production so a stray $PORT from a dev shell can't move the local + // listener. resolveBind likewise binds loopback locally, 0.0.0.0 in prod. + if err := resolvePort(port, portExplicit, prod, os.Getenv("PORT")); err != nil { return err } - resolveBind(bind, bindExplicit, publicMode) + resolveBind(bind, bindExplicit, prod) + + // The auth requirement keys off actual exposure (a non-loopback bind), + // not the env name: binding off-host without auth or read-only is refused + // no matter how we got there (prod default, explicit -bind, etc.). + exposed := !isLoopbackBind(*bind) staticFS, err := fs.Sub(staticFiles, "static") if err != nil { @@ -174,7 +184,12 @@ func run() error { } } - authBuild, err := buildAuthConfig(*authDisabled, publicMode) + authBuild, err := buildAuthConfig(authBuildParams{ + disabled: *authDisabled, + exposed: exposed, + readOnly: *readOnly, + prod: prod, + }) if err != nil { return err } @@ -212,18 +227,19 @@ func run() error { } server := api.NewServer(api.ServerConfig{ - BindAddr: *bind, - Port: *port, - CORSOrigins: parseCSV(*corsOrigins), - APIPrefixes: prefixes, - StaticFS: staticFS, - Registry: reg, - Auth: authCfg, - ReadOnly: *readOnly, - RepoStore: repoStore, - ReposRoot: absReposRoot, - RepoSyncTokens: appProvider, - SyncInterval: *syncInterval, + BindAddr: *bind, + Port: *port, + CORSOrigins: parseCSV(*corsOrigins), + APIPrefixes: prefixes, + StaticFS: staticFS, + Registry: reg, + Auth: authCfg, + ReadOnly: *readOnly, + RepoStore: repoStore, + ReposRoot: absReposRoot, + RepoSyncTokens: appProvider, + SyncInterval: *syncInterval, + GitHubWebhookSecret: strings.TrimSpace(os.Getenv("STACKIT_GITHUB_WEBHOOK_SECRET")), }) errCh := make(chan error, 1) @@ -277,12 +293,26 @@ func envBool(name string) bool { return err == nil && v } -// envDuration parses the named environment variable as a Go duration (e.g. -// "60s"). Unset or unparseable values yield 0, which disables the sync loop. -func envDuration(name string) time.Duration { - d, err := time.ParseDuration(strings.TrimSpace(os.Getenv(name))) +// defaultSyncInterval is the background mirror-fetch cadence used when +// STACKIT_SYNC_INTERVAL is unset. It exists so the webhook backstop is present +// out of the box: even if a delivery is missed (or a push only touched stackit +// metadata refs, which GitHub sends no push event for), the loop catches up +// within this window. Operators tune it down for fresher state or set 0 to +// disable polling and rely solely on webhooks. +const defaultSyncInterval = 5 * time.Minute + +// envSyncInterval resolves the sync-loop interval default: the value of +// STACKIT_SYNC_INTERVAL when set ("0" explicitly disables the loop), otherwise +// defaultSyncInterval. An unparseable value falls back to the default rather +// than silently disabling the loop. +func envSyncInterval() time.Duration { + raw := strings.TrimSpace(os.Getenv("STACKIT_SYNC_INTERVAL")) + if raw == "" { + return defaultSyncInterval + } + d, err := time.ParseDuration(raw) if err != nil { - return 0 + return defaultSyncInterval } return d } diff --git a/apps/server/port.go b/apps/server/port.go index 47ed8b4b9..2e2aa66a6 100644 --- a/apps/server/port.go +++ b/apps/server/port.go @@ -7,12 +7,18 @@ import ( ) // resolvePort applies a $PORT fallback when -port wasn't passed explicitly. -// flagPort is the destination flag pointer. When portExplicit is true the -// env value is ignored (the operator wins). An empty env is a no-op. -func resolvePort(flagPort *int, portExplicit bool, env string) error { +// flagPort is the destination flag pointer. When portExplicit is true the env +// value is ignored (the operator wins). $PORT is the PaaS convention, so it is +// honored only in production (prod); locally it is ignored, so a stray $PORT +// exported by a dev shell (cmux, Heroku toolbelt, ...) can't move the listener. +// An empty env is a no-op. +func resolvePort(flagPort *int, portExplicit, prod bool, env string) error { if portExplicit { return nil } + if !prod { + return nil + } env = strings.TrimSpace(env) if env == "" { return nil diff --git a/apps/server/port_test.go b/apps/server/port_test.go index 8cb7f467c..379015fba 100644 --- a/apps/server/port_test.go +++ b/apps/server/port_test.go @@ -13,47 +13,67 @@ func TestResolvePort(t *testing.T) { name string start int portExplicit bool + prod bool env string wantPort int wantErrContain string }{ { - name: "no env keeps default", + name: "prod no env keeps default", start: 8080, + prod: true, env: "", wantPort: 8080, }, { - name: "env populates default", + name: "prod env populates default", start: 8080, + prod: true, env: "3000", wantPort: 3000, }, { name: "explicit flag wins over env", start: 9999, + prod: true, env: "3000", wantPort: 9999, portExplicit: true, }, { - name: "whitespace env is treated as unset", + name: "prod whitespace env is treated as unset", start: 8080, + prod: true, env: " ", wantPort: 8080, }, { - name: "invalid env returns error", + name: "prod invalid env returns error", start: 8080, + prod: true, env: "not-a-number", wantErrContain: "invalid PORT env", }, + { + name: "local ignores env", + start: 8080, + prod: false, + env: "3000", + wantPort: 8080, + }, + { + name: "local ignores invalid env without error", + start: 8080, + prod: false, + env: "not-a-number", + wantPort: 8080, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() port := tt.start - err := resolvePort(&port, tt.portExplicit, tt.env) + err := resolvePort(&port, tt.portExplicit, tt.prod, tt.env) if tt.wantErrContain != "" { require.Error(t, err) require.Contains(t, err.Error(), tt.wantErrContain) diff --git a/apps/server/sync_interval_test.go b/apps/server/sync_interval_test.go new file mode 100644 index 000000000..763dc2bc6 --- /dev/null +++ b/apps/server/sync_interval_test.go @@ -0,0 +1,34 @@ +package main + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestEnvSyncInterval(t *testing.T) { + tests := []struct { + name string + set bool + val string + want time.Duration + }{ + {name: "unset defaults to backstop", set: false, want: defaultSyncInterval}, + {name: "explicit value is honored", set: true, val: "60s", want: time.Minute}, + {name: "zero explicitly disables", set: true, val: "0", want: 0}, + {name: "unparseable falls back to default", set: true, val: "garbage", want: defaultSyncInterval}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.set { + t.Setenv("STACKIT_SYNC_INTERVAL", tt.val) + } else { + // t.Setenv with an empty value still marks it set; to test the + // unset path, clear it for this subtest. + t.Setenv("STACKIT_SYNC_INTERVAL", "") + } + require.Equal(t, tt.want, envSyncInterval()) + }) + } +} diff --git a/apps/web/src/app/[[...slug]]/page.tsx b/apps/web/src/app/[[...slug]]/page.tsx new file mode 100644 index 000000000..f81c3c975 --- /dev/null +++ b/apps/web/src/app/[[...slug]]/page.tsx @@ -0,0 +1,13 @@ +import { AppShell } from "@/components/app-shell"; + +// `output: 'export'` emits a single index.html for the empty slug. Every deeper +// path (/{owner}/{repo}/...) is matched by this optional catch-all and handled +// client-side; the Go server serves the same index.html for those deep links on +// hard refresh (SPA fallback in internal/api/static.go). +export function generateStaticParams() { + return [{ slug: [] }]; +} + +export default function Page() { + return ; +} diff --git a/apps/web/src/app/page.tsx b/apps/web/src/components/app-shell.tsx similarity index 60% rename from apps/web/src/app/page.tsx rename to apps/web/src/components/app-shell.tsx index 841f98d90..7ab003d93 100644 --- a/apps/web/src/app/page.tsx +++ b/apps/web/src/components/app-shell.tsx @@ -1,13 +1,13 @@ "use client"; -import { Suspense } from "react"; -import { useSearchParams } from "next/navigation"; +import { usePathname } from "next/navigation"; import { AuthProvider } from "@/components/providers/auth-provider"; import { ConfigProvider } from "@/components/providers/config-provider"; import { RepoProvider } from "@/components/providers/repo-provider"; import { RepoPicker } from "@/components/repo-picker/repo-picker"; import { RepoView } from "@/components/repo-picker/repo-view"; import { ReadOnlyBanner } from "@/components/read-only-banner"; +import { parseRepoPath } from "@/lib/repo-route"; import type { ConfigResponse } from "@/lib/api"; // The server's /api/v1/config endpoint is authoritative for read-only and @@ -23,22 +23,21 @@ const CONFIG_FALLBACK: ConfigResponse = { authRequired: !AUTH_DISABLED, }; -// Single root page driving both the unscoped picker and the per-repo view. +// Client shell driving both the unscoped picker and the per-repo view. // -// The Next.js build is `output: 'export'`, so only the root URL is emitted. -// We scope to a repo via the `?repo=` query string rather than a path -// segment — that way the same static index.html handles every repo in both -// `next dev` and the embedded production server, with no per-repo route to -// pre-generate. +// The Next.js build is `output: 'export'`, so only index.html is emitted; the +// optional catch-all route ([[...slug]]) lets that one shell handle every path. +// We scope to a repo via the path — GitHub-style `/{owner}/{repo}/...` — parsed +// here from usePathname, with the Go server's SPA fallback serving the same +// shell for deep links on refresh (see internal/api/static.go). function Home() { - const params = useSearchParams(); - const repoId = params.get("repo") ?? ""; + const { owner, repo } = parseRepoPath(usePathname()); return ( - {repoId ? ( - + {owner && repo ? ( + ) : ( @@ -48,14 +47,12 @@ function Home() { ); } -export default function Page() { +export function AppShell() { return ( - - - - - - - + + + + + ); } diff --git a/apps/web/src/components/branch-detail/branch-diff.tsx b/apps/web/src/components/branch-detail/branch-diff.tsx index 2f30a02dd..57aaf256f 100644 --- a/apps/web/src/components/branch-detail/branch-diff.tsx +++ b/apps/web/src/components/branch-detail/branch-diff.tsx @@ -226,7 +226,7 @@ function parseConventionalCommit(message: string): ConventionalCommit | null { } export function BranchDiff({ branchName, revision, commits, onExit }: BranchDiffProps) { - const { repoId } = useRepo(); + const { repoRef } = useRepo(); const [diff, setDiff] = useState(null); const [loading, setLoading] = useState(true); const [error, setError] = useState(null); @@ -246,7 +246,7 @@ export function BranchDiff({ branchName, revision, commits, onExit }: BranchDiff setDiff(null); }); - fetchBranchDiff(repoId, branchName) + fetchBranchDiff(repoRef, branchName) .then((result) => { if (!active) return; setDiff(result); @@ -263,7 +263,7 @@ export function BranchDiff({ branchName, revision, commits, onExit }: BranchDiff return () => { active = false; }; - }, [branchName, revision, repoId]); + }, [branchName, revision, repoRef]); const parsed = useMemo(() => { if (!diff?.patch.trim()) { diff --git a/apps/web/src/components/branch-detail/stack-detail.tsx b/apps/web/src/components/branch-detail/stack-detail.tsx index 29c4a36b7..1d2087d04 100644 --- a/apps/web/src/components/branch-detail/stack-detail.tsx +++ b/apps/web/src/components/branch-detail/stack-detail.tsx @@ -30,7 +30,7 @@ export function StackDetailPanel({ const status = stackStatusInfo[stack.status] || stackStatusInfo.incomplete; const issues = collectIssues(stack.branches); const stats = computeStackStats(stack.branches); - const { refresh, repoId } = useRepo(); + const { refresh, repoRef } = useRepo(); const { readOnly } = useConfig(); const allHavePRs = stack.branches.every((b) => b.pr != null); @@ -42,7 +42,7 @@ export function StackDetailPanel({ setSubmitting(true); setSubmitResult(null); try { - const result = await submitStack(repoId, stack.rootBranch); + const result = await submitStack(repoRef, stack.rootBranch); setSubmitResult(result); refresh(); } catch (err) { diff --git a/apps/web/src/components/providers/repo-provider.tsx b/apps/web/src/components/providers/repo-provider.tsx index 7ab2c65ef..89732aac9 100644 --- a/apps/web/src/components/providers/repo-provider.tsx +++ b/apps/web/src/components/providers/repo-provider.tsx @@ -4,6 +4,7 @@ import { createContext, useContext, useEffect, + useMemo, useState, useCallback, useRef, @@ -11,6 +12,7 @@ import { } from "react"; import { fetchView, + type RepoRef, type RepoResponse, type StackDetail, type TrunkCommitResponse, @@ -23,7 +25,7 @@ import { diffViews } from "@/lib/diff-views"; const MAX_EVENTS = 100; interface RepoState { - repoId: string; + repoRef: RepoRef; repo: RepoResponse | null; stackDetails: StackDetail[]; recentlyMerged: TrunkCommitResponse[]; @@ -43,7 +45,17 @@ export function useRepo() { return ctx; } -export function RepoProvider({ repoId, children }: { repoId: string; children: ReactNode }) { +export function RepoProvider({ + owner, + repo: repoName, + children, +}: { + owner: string; + repo: string; + children: ReactNode; +}) { + // Stable ref so the load/SSE effects don't re-run on every render. + const repoRef = useMemo(() => ({ owner, repo: repoName }), [owner, repoName]); const [repo, setRepo] = useState(null); const [stackDetails, setStackDetails] = useState([]); const [recentlyMerged, setRecentlyMerged] = useState
([]); @@ -69,115 +81,17 @@ export function RepoProvider({ repoId, children }: { repoId: string; children: R const loadData = useCallback(async () => { try { - const view = await fetchView(repoId); + const view = await fetchView(repoRef); setRepo(view.repo); - // TODO: Remove sample stacks — for UI development only - const sampleStacks: StackDetail[] = [ - { - rootBranch: "sample/auth-refactor", - title: "Refactor auth middleware", - status: "shippable", - branchCount: 2, - prCount: 2, - isCurrent: false, - owner: "teammate-alice", - branches: [ - { - name: "sample/auth-refactor", - depth: 0, - isCurrent: false, - needsRestack: false, - isLocked: false, - isFrozen: false, - revision: "abc1234", - commitDate: new Date().toISOString(), - commitAuthor: "teammate-alice", - commitCount: 3, - linesAdded: 120, - linesDeleted: 45, - pr: { - number: 101, - title: "Refactor auth middleware", - state: "OPEN", - url: "#", - isDraft: false, - base: "main", - }, - }, - { - name: "sample/auth-tests", - parent: "sample/auth-refactor", - depth: 1, - isCurrent: false, - needsRestack: false, - isLocked: false, - isFrozen: false, - revision: "def5678", - commitDate: new Date().toISOString(), - commitAuthor: "teammate-alice", - commitCount: 1, - linesAdded: 80, - linesDeleted: 0, - pr: { - number: 102, - title: "Add auth middleware tests", - state: "OPEN", - url: "#", - isDraft: false, - base: "sample/auth-refactor", - }, - }, - ], - }, - { - rootBranch: "sample/fix-pagination", - title: "Fix pagination off-by-one", - status: "pending", - branchCount: 1, - prCount: 1, - isCurrent: false, - owner: "teammate-bob", - branches: [ - { - name: "sample/fix-pagination", - depth: 0, - isCurrent: false, - needsRestack: true, - isLocked: false, - isFrozen: false, - revision: "fed9876", - commitDate: new Date().toISOString(), - commitAuthor: "teammate-bob", - commitCount: 1, - linesAdded: 5, - linesDeleted: 3, - pr: { - number: 200, - title: "Fix pagination off-by-one", - state: "OPEN", - url: "#", - isDraft: true, - base: "main", - }, - }, - ], - }, - ]; - - const augmentedView: ViewResponse = { - repo: view.repo, - stacks: [...view.stacks, ...sampleStacks], - }; - // Diff against previous view to detect changes if (prevViewRef.current) { - const detected = diffViews(prevViewRef.current, augmentedView); + const detected = diffViews(prevViewRef.current, view); addEvents(detected); } - prevViewRef.current = augmentedView; + prevViewRef.current = view; - setStackDetails(augmentedView.stacks); + setStackDetails(view.stacks); setRecentlyMerged(view.recentlyMerged ?? []); setError(null); setLastUpdated(new Date()); @@ -186,7 +100,7 @@ export function RepoProvider({ repoId, children }: { repoId: string; children: R } finally { setLoading(false); } - }, [addEvents, repoId]); + }, [addEvents, repoRef]); // Initial load useEffect(() => { @@ -198,12 +112,12 @@ export function RepoProvider({ repoId, children }: { repoId: string; children: R }, [loadData]); // SSE updates trigger refresh; server events get added directly - useSSE(repoId, loadData, addEvent); + useSSE(repoRef, loadData, addEvent); return ( { expect(screen.getByText("Other Project")).toBeDefined(); }); - it("navigates to /?repo={repoId} on click", async () => { + it("navigates to /{owner}/{repo} on click", async () => { mockRepos({ - repos: [{ id: "stackit", displayName: "Stackit", trunk: "main" }], + repos: [{ id: "stackit", owner: "acme", repo: "web", displayName: "Stackit", trunk: "main" }], }); render(); const card = await screen.findByTestId("repo-card-stackit"); fireEvent.click(card); - expect(mockPush).toHaveBeenCalledWith("/?repo=stackit"); + expect(mockPush).toHaveBeenCalledWith("/acme/web"); }); it("shows the empty-state message when no repos are configured", async () => { diff --git a/apps/web/src/components/repo-picker/repo-picker.tsx b/apps/web/src/components/repo-picker/repo-picker.tsx index 03b779135..2d1cb3bc4 100644 --- a/apps/web/src/components/repo-picker/repo-picker.tsx +++ b/apps/web/src/components/repo-picker/repo-picker.tsx @@ -10,6 +10,7 @@ import { CardTitle, } from "@/components/ui/card"; import { fetchRepos, type RepoSummary } from "@/lib/api"; +import { buildRepoPath } from "@/lib/repo-route"; import { AddRepository } from "./add-repository"; export function RepoPicker() { @@ -17,8 +18,12 @@ export function RepoPicker() { const [repos, setRepos] = useState(null); const [error, setError] = useState(null); - const openRepo = (id: string) => - router.push(`/?repo=${encodeURIComponent(id)}`); + // Repos are addressed by their GitHub coordinates. A repo with no remote has + // none and can't be opened in the path-based UI. + const openRepo = (repo: RepoSummary) => { + if (!repo.owner || !repo.repo) return; + router.push(buildRepoPath(repo.owner, repo.repo, null)); + }; useEffect(() => { let active = true; @@ -65,7 +70,7 @@ export function RepoPicker() { Add a GitHub repository to get started.

- openRepo(repo.id)} /> + openRepo(repo)} />
); } @@ -86,7 +91,7 @@ export function RepoPicker() {