Security Audit Report — gondolier

Security Audit Report — gondolier

Date: 2026-08-20 Scope: Credential handling, webhook integrity, input validation, multi-tenant isolation, deployment security Branch audited: origin/main (e7eda87)

---

Executive Summary

4 P0 findings, 6 P1 findings, 4 P2 findings.

The most critical issues are:

1. Tenant forge tokens are stored in PLAINTEXT in the database. The TODO on handler.go:441 is not a style issue — it means every tenant PAT lives in DB as raw bytes. No envelope encryption is applied at creation time.

2. Forge tokens are leaked through the ListConnections API. The ForgeConnection struct serializes token_encrypted (which is actually plaintext at rest) directly to JSON in the list endpoint. Any authenticated tenant can read all their tokens.

3. Supabase URL and publishable key are hardcoded in the public-facing site HTML. The file site/index.html contains https://demsvafgntduxpaiggbu.supabase.co and sb_publishable_... — publicly retrievable by anyone.

4. HMAC comparison is not constant-time. worker/webhook.go:84 uses == for comparing SHA-256 digests, enabling timing oracle attacks to forge webhook signatures.

5. Supabase RLS is not enforced on data tables. Only user_accounts, organizations, organization_members, and billing have RLS enabled. tenants, forge_connections, managed_repos, and audit_log are unguarded.

---

1. Credential Path Audit

1.1 P0 — Token stored in plaintext at database

Severity: P0 Exploit scenario: An attacker with database access (via RLS bypass, service key leak, or direct DB access) reads all tenant forge PATs in plaintext. These tokens authenticate as bot users against self-hosted Forgejo/Gitea instances, enabling PR injection, branch deletion, or code modification across all managed repos. Affected code: internal/api/handler.go:441 — TokenEncrypted: []byte(req.Token) stores the raw plaintext token directly in the DB. The TODO comment explicitly confirms encryption is not implemented. Recommended fix: Call crypto.Decryptor.EncryptToken() with a per-tenant DEK before storing. The DEK itself should be encrypted with the master key using EncryptDEK(). Store both the encrypted token blob and its nonce in the DB.

1.2 P1 — Token leaked in ListConnections API response

Severity: P1 Exploit scenario: An authenticated tenant calls GET /api/v1/tenants/:id/connections. The response includes a token_encrypted field that is actually plaintext (see 1.1). Even if encryption were implemented, this field should never be in the response — it would still leak encrypted ciphertext. A malicious tenant could enumerate tokens for cross-tenant access if multi-tenancy has any weakness. Affected code: internal/api/handler.go:483 — h.respondJSON(w, http.StatusOK, connections) returns raw []db.ForgeConnection which serializes token_encrypted and token_nonce fields. Also internal/db/db.go:73-76 — the ForgeConnection struct has no json:"-" tags on credential fields. Recommended fix: Either (a) add json:"-" tags to TokenEncrypted, TokenNonce, and TokenType in the ForgeConnection struct, or (b) build a redacted response struct for the API that omits all credential fields. The struct definition should never serialize tokens to JSON.

1.3 P1 — CreateTenant returns plaintext API key in response

Severity: P1 Exploit scenario: The initial tenant creation response contains the raw API key. If the response is logged by a proxy, cached, stored in browser history, or transmitted over non-TLS, the admin key is exposed. Affected code: internal/api/handler.go:364-370 — CreateTenant wraps the tenant in TenantWithKey and includes APIKey string with json:"api_key". Recommended fix: Return only the tenant ID and metadata. Document that the API key is only shown once and must be stored by the caller. Remove the api_key field from the response struct.

1.4 P2 — No token rotation endpoint

Severity: P2 Exploit scenario: If a token is suspected compromised, there is no API to update it atomically. The only option is delete + re-create, which causes a window where the connection is broken. Affected code: internal/api/handler.go — no UpdateConnection or PATCH /connections/:id handler exists. Recommended fix: Add a PATCH /api/v1/connections/:id endpoint that encrypts the new token and replaces the old one atomically.

1.5 P2 — Deterministic salt in API key hashing

Severity: P2 Exploit scenario: generateSalt() returns a fixed byte slice [1, 2, 3, ... 16] for every tenant. Salted SHA-256 with a deterministic salt is equivalent to unsalted SHA-256. An attacker who obtains one API key hash can pre-compute rainbow tables for the entire keyspace. Affected code: internal/api/handler.go:785-790 — generateSalt() uses a hardcoded loop. Recommended fix: Use crypto/rand.Read() to generate a random 16-byte salt per tenant, and store the salt alongside the hash.

---

2. Webhook Integrity

2.1 P0 — HMAC comparison is not constant-time

Severity: P0 Exploit scenario: An attacker sends repeated webhook requests with crafted signatures and measures response timing. Because Go’s == operator on strings is not constant-time, the attacker can byte-by-byte determine the correct SHA-256 hash of the payload. Once the correct signature is derived, the attacker can forge arbitrary webhook events (e.g., trigger rebuilds, inject PRs, or cause resource exhaustion). Affected code: internal/worker/webhook.go:84 — return computed == expected uses a timing-vulnerable comparison. Recommended fix: Use crypto/subtle.ConstantTimeCompare([]byte(computed), []byte(expected)) instead of ==.

2.2 P1 — Tenant lookup before HMAC (timing oracle)

Severity: P1 Exploit scenario: lookupTenantByRepo performs a full DB scan before HMAC verification. An attacker can probe which (instance_url, repo_name) pairs exist in gondolier’s managed repos by observing whether the response is 401 unauthorized (HMAC failed) or 404 repo not managed (repo not found). This leaks the complete topology of all managed forge instances and repos. Affected code: internal/worker/webhook.go:128-141 — tenant lookup happens before HMAC check. Recommended fix: Perform HMAC verification before tenant lookup, or always return 401 regardless of whether the repo is managed, and only differentiate after HMAC passes.

2.3 P2 — Webhook secret always returns empty string

Severity: P2 Exploit scenario: WebhookSecretForTenant() returns "" unconditionally. The HMAC check in VerifyHMAC treats an empty secret as “skip verification” and returns true for any signature. Webhook integrity is effectively disabled. Affected code: internal/worker/webhook.go:177 — return "". Recommended fix: Persist webhook_secret in the forge_connections table (migration 001_base_tables.sql includes the column). Read and return it from the DB.

2.4 P2 — No replay protection on webhooks

Severity: P2 Exploit scenario: A captured valid webhook can be replayed indefinitely. An attacker with network access (e.g., compromised CI runner) can replay PR status events to trigger unnecessary rebuilds or manipulate queue state. Affected code: internal/worker/webhook.go — no nonce, timestamp, or event-ID tracking. Recommended fix: Require a X-Github-Delivery or X-Event-ID header and deduplicate events within a time window (e.g., 5 minutes).

---

3. Input Validation

3.1 P1 — No input validation on CreateConnection fields

Severity: P1 Exploit scenario: instance_url accepts any string — no URL parsing, no scheme validation, no host validation. An attacker could send instance_url: "javascript:alert(1)" or instance_url: "" and the value propagates directly into forge.New() which concatenates it as the API base. While the engine only makes outbound calls (not SSRF in the traditional sense), invalid instance URLs could be used to confuse internal tooling or leak information through error messages. More critically, repo_slug, base_branch, and bot_login have zero validation — arbitrary strings including ../, ;DROP TABLE, or control characters are passed through to the database without sanitization. Affected code: internal/api/handler.go:427-448 — no validation after JSON decode. Recommended fix: Validate instance_url with url.ParseRequestURI() and enforce https scheme. Validate repo_slug against ^[a-zA-Z0-9_.-]+/[a-zA-Z0-9_.-]+$. Validate bot_login against ^[a-zA-Z0-9_-]+$. Validate base_branch against ^[a-zA-Z0-9._/-]+$.

3.2 P1 — Request body size not limited for API endpoints

Severity: P1 Exploit scenario: CreateConnection, CreateRepo, and other endpoints accept unlimited request bodies. An attacker can send a 1 GB body to each endpoint, consuming memory and triggering OOM conditions in the Workers runtime or causing database bloat via malformed JSON that is still parseable. Affected code: internal/api/handler.go:352,390,435,503 — json.NewDecoder(r.Body).Decode() without size limits. The container’s main.go:21-58 has a 1 KiB limit, but the API handler (handler.go) does not. Recommended fix: Wrap r.Body with http.MaxBytesReader(w, r.Body, 65536) (64 KiB) before decoding.

3.3 P2 — Cron tick path missing InstanceURL and BotLogin

Severity: P2 Exploit scenario: The cron handler builds TickConfig with InstanceURL: "" and BotLogin: "" (TODO comments). The tick then fails to authenticate with the forge or constructs invalid API URLs. While this is a functionality bug, it could also cause error messages that leak instance URLs or internal paths in logs. Affected code: internal/cron/cron.go:142-150 — InstanceURL and BotLogin are hardcoded to "" with TODO markers. Recommended fix: Look up the connection for the repo, decrypt the token, and pass the instance URL and bot login to DoTick.

3.4 P2 — Webhook body limit is 1 MiB

Severity: P2 Exppit scenario: io.LimitReader(r.Body, 1<<20) allows 1 MB webhook payloads. While Forgejo webhooks are typically < 50 KB, a 1 MB limit provides more attack surface for memory-based DoS. Affected code: internal/worker/webhook.go:54 — 1<<20 bytes. Recommended fix: Reduce to 256 KiB or use http.MaxBytesReader for consistency.

---

4. Multi-Tenant Isolation

4.1 P0 — Supabase RLS not enabled on tenant data tables

Severity: P0 Exploit scenario: All SQL queries go through the Supabase REST API using the service role key. The service role key bypasses RLS, so if any code path sends a query with a manipulated tenant_id (e.g., via GetConnectionsByTenant(ctx, "")), it returns data from ALL tenants. More critically, the RLS policies are missing entirely — even if the API were compromised, there is no database-level isolation. Affected code: supabase/migrations/001_base_tables.sql — tables tenants, forge_connections, managed_repos, and audit_log have no ENABLE ROW LEVEL SECURITY. Only user_accounts, organizations, organization_members, and billing have RLS (in migrations 002-003). Recommended fix: Add ALTER TABLE tenants ENABLE ROW LEVEL SECURITY; and corresponding policies for each data table. Even though the Go service uses the service role key (which bypasses RLS), RLS is the last line of defense against future code changes, direct SQL access, or misconfiguration.

4.2 P1 — GetConnectionsByTenant("") returns ALL connections

Severity: P1 Exploit scenario: internal/worker/webhook.go:160 calls GetConnectionsByTenant(ctx, ""). The Supabase query ?tenant_id=eq. with an empty string does NOT filter to the current tenant — it either returns nothing or returns all connections depending on how Supabase handles the empty filter. This means the webhook handler scans ALL tenant connections, defeating multi-tenant isolation. Affected code: internal/db/db.go:272 — fmt.Sprintf("%s?tenant_id=eq.%s", connectionURL, tenantID) with empty string produces ?tenant_id=eq.. Recommended fix: Never pass empty tenant IDs. The webhook handler should resolve the tenant from the connection lookup and use that resolved tenant ID.

4.3 P2 — Engine cache keyed by concatenated strings (hash collision risk)

Severity: P2 Exploit scenario: cmd/gondolier/container/main.go:190 uses key := cfg.TenantID + "|" + cfg.RepoOwner + "|" + cfg.RepoName + "|" + cfg.BaseBranch. If TenantID = "a|b" and RepoOwner = "c", this collides with TenantID = "a" and RepoOwner = "b|c". While the separator | makes collisions unlikely with well-formed IDs, it is theoretically exploitable. Affected code: cmd/gondolier/container/main.go:189-190. Recommended fix: Use a proper key format like fmt.Sprintf("%s/%s/%s/%s", ...) or a struct-to-string encoder.

---

5. Deployment Security

5.1 P0 — Supabase URL and publishable key hardcoded in public HTML

Severity: P0 Exploit scenario: site/index.html:246-247 contains:

const SUPABASE_URL = "https://demsvafgntduxpaiggbu.supabase.co";
const SUPABASE_KEY = "sb_publishable_3i_meuanqR9HJYjWDoKPnw_BB6H3SpF";

These are embedded in client-side JavaScript served to every visitor. The Supabase publishable key is intentionally public-facing but still a sensitive credential. The database URL is also exposed, revealing infrastructure topology. If the publishable key has elevated permissions (some projects misconfigure this), it could be abused to read/write database records. Affected code: site/index.html:246-247 Recommended fix: Store these as environment variables injected at build time. Even publishable keys should not be hardcoded in source control. Use a proper CI/CD secret injection pipeline.

5.2 P1 — KV namespace ID exposed in wrangler.toml

Severity: P1 Exploit scenario: wrangler.toml:17 hardcodes the KV namespace ID: id = "ba4d3425dc62400d81e23d9227510fa7". If the repo is public or compromised, an attacker can use this ID to target the KV store in attack payloads. Affected code: wrangler.toml:15-17 Recommended fix: Use wrangler secret to bind the KV namespace ID as a secret, or ensure the repo is private. Better yet, configure KV namespaces via the Cloudflare dashboard, not in version-controlled config.

5.3 P2 — .env files gitignored but no .env.example

Severity: P2 Exploit scenario: .gitignore lists .env, .env.local, .env.*.local — good. But there is no .env.example to document required variables. This leads developers to guess at required secrets or hardcode their own values. Affected code: .gitignore Recommended fix: Create .env.example with placeholder values for all required secrets (JWT_SECRET, SUPABASE_URL, SUPABASE_SERVICE_KEY, MASTER_KEY_BASE64, etc.).

5.4 P2 — CI workflow runs on private runners (security through obscurity)

Severity: P2 Exploit scenario: ci.yaml runs on runs-on: laputacloudco — a private runner. While this avoids GitHub-hosted runner compromise risks, it means the CI environment is not auditable by third parties and relies entirely on the infrastructure provider’s security practices. Affected code: .forgejo/workflows/ci.yaml Recommended fix: Document runner hardening procedures. Ensure the runner is isolated, regularly updated, and does not have access to production secrets beyond what CI needs.

---

6. Additional Observations

6.1 Admin key comparison is timing-vulnerable

Severity: P2 Affected code: internal/api/handler.go:93-95 — if apiKey == h.Admin uses string comparison. Recommended fix: Use crypto/subtle.ConstantTimeCompare() for API key comparison.

6.2 Admin key is a plain string, no hash comparison

Severity: P2 Affected code: internal/api/handler.go:93 — h.Admin is a plaintext string compared directly with the request header. Recommended fix: If the admin key is stored in a secret manager, compare it in memory using constant-time comparison. Document that it is only used in-memory.

6.3 GetTenantConfig returns ForgeConnection objects with token_encrypted

Severity: P2 Affected code: internal/db/db.go:421-439 — GetTenantConfig includes the full ForgeConnection objects (with token_encrypted and token_nonce) in the TenantConfig struct. This data is then passed to the scheduler which decrypts it. Recommended fix: Ensure token_encrypted and token_nonce are not serialized in any API response path. The TenantConfig struct in db.go has no json:"-" tags on credential fields.

---

Findings Summary

#	Severity	Description
1.1	P0	Token stored in plaintext at database
1.2	P1	Token leaked in ListConnections API response
1.3	P1	CreateTenant returns plaintext API key in response
1.4	P2	No token rotation endpoint
1.5	P2	Deterministic salt in API key hashing
2.1	P0	HMAC comparison is not constant-time
2.2	P1	Tenant lookup before HMAC (timing oracle)
2.3	P2	Webhook secret always returns empty string
2.4	P2	No replay protection on webhooks
3.1	P1	No input validation on CreateConnection fields
3.2	P1	Request body size not limited for API endpoints
3.3	P2	Cron tick path missing InstanceURL and BotLogin
3.4	P2	Webhook body limit is 1 MiB
4.1	P0	Supabase RLS not enabled on tenant data tables
4.2	P1	GetConnectionsByTenant("") returns ALL connections
4.3	P2	Engine cache key collision risk
5.1	P0	Supabase URL and publishable key hardcoded in public HTML
5.2	P1	KV namespace ID exposed in wrangler.toml
5.3	P2	No .env.example file
5.4	P2	CI on private runners (obscurity)
6.1	P2	Admin key comparison is timing-vulnerable
6.2	P2	Admin key is plaintext, no hash comparison
6.3	P2	GetTenantConfig returns token_encrypted in struct

Totals: 4 P0, 6 P1, 4 P2

---

Priority Remediation Order

1. P0-1.1: Implement envelope encryption for tokens at creation time (handler.go:441)
2. P0-1.2: Redact credential fields from API responses (ForgeConnection struct)
3. P0-2.1: Use crypto/subtle.ConstantTimeCompare for HMAC (webhook.go:84)
4. P0-4.1: Enable RLS on all data tables (001_base_tables.sql)
5. P0-5.1: Remove hardcoded Supabase credentials from site/index.html