diff --git a/.env.example b/.env.example index 6b78df3..efd6a45 100644 --- a/.env.example +++ b/.env.example @@ -6,9 +6,9 @@ COMPOSE_PROJECT_NAME=wiredraft WIREDRAFT_IMAGE=ghcr.io/arumes31/wiredraft:latest POSTGRES_IMAGE=postgres:17-alpine@sha256:742f40ea20b9ff2ff31db5458d127452988a2164df9e17441e191f3b72252193 -# 0.0.0.0 publishes WireDraft on every host interface. Use 127.0.0.1 when only -# a local reverse proxy should reach it. -WIREDRAFT_BIND_ADDRESS=0.0.0.0 +# WireDraft serves plain HTTP. The loopback default lets only a TLS reverse +# proxy on this host reach it; use 0.0.0.0 to publish on every interface. +WIREDRAFT_BIND_ADDRESS=127.0.0.1 WIREDRAFT_PUBLISHED_PORT=8080 # Bundled PostgreSQL container. diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index cb65c3b..c86a84e 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -53,7 +53,7 @@ jobs: - name: Set up Node.js uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: - node-version: 24 + node-version: "24.12.0" cache: npm - name: Check JavaScript syntax run: find web scripts e2e -type f \( -name '*.js' -o -name '*.mjs' \) -print0 | xargs -0 -r -n1 node --check diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index c435b0d..300aaac 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -60,7 +60,7 @@ jobs: - name: Set up Node.js uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: - node-version: 24 + node-version: "24.12.0" cache: npm - name: Install locked dependencies run: npm ci --ignore-scripts @@ -119,7 +119,7 @@ jobs: - name: Set up Node.js uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: - node-version: 24 + node-version: "24.12.0" cache: npm - name: Install locked dependencies run: npm ci --ignore-scripts @@ -170,7 +170,7 @@ jobs: - name: Set up Node.js uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: - node-version: 24 + node-version: "24.12.0" cache: npm - name: Install locked dependencies run: npm ci --ignore-scripts @@ -217,7 +217,7 @@ jobs: - name: Set up Node.js for frontend minification uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: - node-version: 24 + node-version: "24.12.0" package-manager-cache: false - name: Prepare minified JavaScript before image build run: | diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 7b75c84..498906e 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -38,7 +38,7 @@ jobs: - name: Set up Node.js uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: - node-version: 24 + node-version: "24.12.0" cache: npm - name: Install pinned scanners run: | @@ -115,7 +115,7 @@ jobs: - name: Set up Node.js for frontend minification uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: - node-version: 24 + node-version: "24.12.0" package-manager-cache: false - name: Prepare minified JavaScript before image build run: | diff --git a/.github/workflows/strix-penetration-test.yml b/.github/workflows/strix-penetration-test.yml index de3209c..8ffd6a9 100644 --- a/.github/workflows/strix-penetration-test.yml +++ b/.github/workflows/strix-penetration-test.yml @@ -30,7 +30,7 @@ jobs: STRIX_SANDBOX_CONNECT_TIMEOUT: "3600" steps: - name: Check out repository - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 timeout-minutes: 5 with: fetch-depth: 0 @@ -78,7 +78,10 @@ jobs: env: STRIX_LLM: ${{ secrets.STRIX_LLM }} LLM_API_KEY: ${{ secrets.LLM_API_KEY }} - LLM_API_BASE: https://nimbusroute.wowcraft.pw/v1 + # The scan sends repository source and LLM_API_KEY to this endpoint. + # Override it with the STRIX_LLM_API_BASE repository variable to use a + # provider whose data-handling terms the maintainers have reviewed. + LLM_API_BASE: ${{ vars.STRIX_LLM_API_BASE || 'https://nimbusroute.wowcraft.pw/v1' }} STRIX_DIFF_BASE: ${{ github.base_ref }} shell: bash run: | diff --git a/.github/workflows/supply-chain.yml b/.github/workflows/supply-chain.yml index 5d473c3..f1aadab 100644 --- a/.github/workflows/supply-chain.yml +++ b/.github/workflows/supply-chain.yml @@ -39,7 +39,7 @@ jobs: - name: Set up Node.js uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: - node-version: 24 + node-version: "24.12.0" package-manager-cache: false - name: Build minified embedded modules run: | @@ -87,6 +87,9 @@ jobs: container: name: Publish multi-architecture GHCR image + # Only main and release tags publish images. A manual dispatch from any + # other branch would otherwise turn its branch name into a GHCR tag. + if: github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/v') runs-on: ubuntu-latest timeout-minutes: 30 permissions: @@ -100,13 +103,25 @@ jobs: - name: Set up Node.js for frontend minification uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: - node-version: 24 + node-version: "24.12.0" package-manager-cache: false - name: Prepare minified JavaScript before image build run: | npm ci --ignore-scripts npm run minify:js find .quality-data/minified-js -type f -name '*.js' -print0 | xargs -0 -n1 node --check + - name: Verify release tag points at a commit on main + if: startsWith(github.ref, 'refs/tags/v') + env: + GH_TOKEN: ${{ github.token }} + GH_REPO: ${{ github.repository }} + SHA: ${{ github.sha }} + run: | + status="$(gh api "repos/$GH_REPO/compare/main...$SHA" --jq .status)" + if [[ "$status" != "identical" && "$status" != "behind" ]]; then + echo "::error::Release tags must point at a commit already on main (compare status: $status)." + exit 1 + fi - name: Set up QEMU uses: docker/setup-qemu-action@1f40c72289eff860ee54a304f1438e3cff362e0a # v4.3.0 - name: Set up Docker Buildx diff --git a/.github/workflows/visual-regression.yml b/.github/workflows/visual-regression.yml index 8a9a31b..d4f8499 100644 --- a/.github/workflows/visual-regression.yml +++ b/.github/workflows/visual-regression.yml @@ -51,7 +51,7 @@ jobs: - name: Set up Node.js uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: - node-version: 24 + node-version: "24.12.0" cache: npm - name: Install locked dependencies run: npm ci --ignore-scripts diff --git a/README.md b/README.md index 8acdfeb..5e3eab5 100644 --- a/README.md +++ b/README.md @@ -47,7 +47,7 @@ docker compose up --build -d docker compose ps ``` -Open . Session cookies are HTTPS-only by default; browsers accept them on `localhost`, but to sign in over plain HTTP from another host set `WIREDRAFT_COOKIE_SECURE=false` in `.env` (evaluation only). View logs or stop the stack with: +Open . Session cookies are HTTPS-only by default; browsers accept them on `localhost`, but to sign in over plain HTTP from another host set `WIREDRAFT_BIND_ADDRESS=0.0.0.0` and `WIREDRAFT_COOKIE_SECURE=false` in `.env` (evaluation only). The port is published on `127.0.0.1` by default. View logs or stop the stack with: ```sh docker compose logs -f wiredraft @@ -113,7 +113,7 @@ Compose-specific settings are separate from the application configuration below: | `COMPOSE_PROJECT_NAME` | `wiredraft` | Compose project and generated resource-name prefix | | `WIREDRAFT_IMAGE` | `ghcr.io/arumes31/wiredraft:latest` | Application image tag or digest | | `POSTGRES_IMAGE` | pinned PostgreSQL 17 Alpine digest | PostgreSQL image tag or digest | -| `WIREDRAFT_BIND_ADDRESS` | `0.0.0.0` | Host address that publishes the web port; use `127.0.0.1` behind a local reverse proxy | +| `WIREDRAFT_BIND_ADDRESS` | `127.0.0.1` | Host address that publishes the web port. WireDraft serves plain HTTP, so keep the loopback default behind a TLS reverse proxy on the host; set `0.0.0.0` only when another trusted host terminates TLS | | `WIREDRAFT_PUBLISHED_PORT` | `8080` | Host-side web port | | `POSTGRES_DB` | `wiredraft` | Database initialized in a new PostgreSQL data directory | | `POSTGRES_USER` | `wiredraft` | Database owner initialized in a new PostgreSQL data directory | diff --git a/cmd/server/main.go b/cmd/server/main.go index 28389e3..a23a137 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "io/fs" + "log/slog" "net/http" "os" "os/signal" @@ -50,6 +51,11 @@ func run(args []string) error { return err } defer databasePool.Close() + releaseInstanceLock, err := store.AcquireInstanceLock(context.Background(), databasePool) + if err != nil { + return err + } + defer releaseInstanceLock() if err := store.MigrateDatabase(databasePool); err != nil { return fmt.Errorf("migrating database: %w", err) } @@ -79,6 +85,7 @@ func run(args []string) error { appLogger.Error("closing photo storage", "error", err) } }() + sweepOrphanedMedia(context.Background(), topologyStore, mediaStore, appLogger) organizations, err := topologyStore.ListOrganizations(context.Background()) if err != nil { return fmt.Errorf("listing organizations: %w", err) @@ -88,6 +95,9 @@ func run(args []string) error { authOrganizations[index] = auth.OrganizationRef{ID: organization.ID, Name: organization.Name} } secretKey, previousSecretKeys, err := readAuthSecretKeys(cfg.SecretKeyFile, cfg.PreviousSecretKeyFile) + if cfg.SecretKeyFile == "" { + appLogger.Warn("TOTP secrets are sealed with a key stored in the same database row; set WIREDRAFT_AUTH_SECRET_KEY_FILE so a database dump alone cannot open them") + } if err != nil { return err } @@ -269,3 +279,35 @@ func probeHealth(url string) (returnErr error) { } return nil } + +// sweepOrphanedMedia removes photo files that no topology references. It runs +// before the server accepts requests, and it skips the sweep entirely when the +// complete reference set cannot be read. +func sweepOrphanedMedia(ctx context.Context, topologyStore store.Store, mediaStore *media.Store, logger *slog.Logger) { + summaries, err := topologyStore.List(ctx) + if err != nil { + logger.Warn("skipping orphaned media sweep", "error", err) + return + } + var references []media.Reference + for _, summary := range summaries { + topology, err := topologyStore.Get(ctx, summary.ID) + if err != nil { + logger.Warn("skipping orphaned media sweep", "topology_id", summary.ID, "error", err) + return + } + for _, photo := range topology.Photos { + references = append(references, media.Reference{ + TopologyID: topology.ID, PhotoID: photo.ID, MediaType: photo.MediaType, + }) + } + } + removed, err := mediaStore.Sweep(references) + if err != nil { + logger.Warn("orphaned media sweep incomplete", "removed", removed, "error", err) + return + } + if removed > 0 { + logger.Info("removed orphaned media", "count", removed) + } +} diff --git a/docker-compose.ghcr.yml b/docker-compose.ghcr.yml index 7bc8518..fd59d10 100644 --- a/docker-compose.ghcr.yml +++ b/docker-compose.ghcr.yml @@ -25,7 +25,7 @@ services: image: ${WIREDRAFT_IMAGE:-ghcr.io/arumes31/wiredraft:latest} pull_policy: always ports: - - "${WIREDRAFT_BIND_ADDRESS:-0.0.0.0}:${WIREDRAFT_PUBLISHED_PORT:-8080}:${PORT:-8080}" + - "${WIREDRAFT_BIND_ADDRESS:-127.0.0.1}:${WIREDRAFT_PUBLISHED_PORT:-8080}:${PORT:-8080}" environment: PORT: ${PORT:-8080} DATABASE_URL: ${DATABASE_URL:-} diff --git a/docker-compose.yml b/docker-compose.yml index 3356a66..7bb1130 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -22,7 +22,7 @@ services: build: . image: wiredraft:latest ports: - - "8080:8080" + - "${WIREDRAFT_BIND_ADDRESS:-127.0.0.1}:8080:8080" environment: PORT: "8080" PGHOST: postgres diff --git a/e2e/critical-workflows.spec.mjs b/e2e/critical-workflows.spec.mjs index 9d68efd..be5086f 100644 --- a/e2e/critical-workflows.spec.mjs +++ b/e2e/critical-workflows.spec.mjs @@ -390,10 +390,11 @@ test("stores plan comments in the inspector and manages map resources", async ({ const comments = page.locator(".inspector-comments"); await expect(comments).toContainText("PLAN COMMENTS"); await expect(comments).toContainText("STORED WITH THIS MAP"); - await comments.locator('[name="author"]').fill("E2E Operator"); + await expect(comments.locator('[name="author"]')).toHaveValue("Guest"); + await expect(comments.locator('[name="author"]')).toHaveAttribute("readonly", ""); await comments.locator('[name="body"]').fill("Validate the maintenance path before change window."); await comments.locator(".inspector-comment-form button").click(); - await expect(comments).toContainText("E2E Operator"); + await expect(comments.locator(".inspector-comment-list")).toContainText("Guest"); await expect(comments).toContainText("Validate the maintenance path before change window."); const persisted = await request.get(`/api/v1/topologies/${encodeURIComponent(topologyID)}`).then((response) => response.json()); diff --git a/internal/auth/entra.go b/internal/auth/entra.go index b43d55b..ddff9bf 100644 --- a/internal/auth/entra.go +++ b/internal/auth/entra.go @@ -150,11 +150,16 @@ func (p *EntraProvider) Complete( p.mu.Lock() p.pruneFlowsLocked(now) flow, exists := p.flows[state] - delete(p.flows, state) - p.mu.Unlock() providedHash := sha256.Sum256([]byte(flowToken)) - if !exists || state == "" || flowToken == "" || code == "" || - subtle.ConstantTimeCompare(flow.CookieHash[:], providedHash[:]) != 1 { + matches := exists && state != "" && flowToken != "" && + subtle.ConstantTimeCompare(flow.CookieHash[:], providedHash[:]) == 1 + // Only the browser holding the flow cookie may consume the flow, so a + // party that merely learns the state value cannot cancel the login. + if matches { + delete(p.flows, state) + } + p.mu.Unlock() + if !matches || code == "" { return ExternalIdentity{}, ErrInvalidExternalIdentity } runtime, err := p.oidcRuntime(exchangeContext) diff --git a/internal/auth/entra_test.go b/internal/auth/entra_test.go index 05fcaf8..e6aa119 100644 --- a/internal/auth/entra_test.go +++ b/internal/auth/entra_test.go @@ -230,3 +230,20 @@ func TestEntraHomeTenantMemberRequiresAccountTypeClaimWhenConfigured(t *testing. } } } + +func TestEntraFlowIsNotConsumedByAWrongCookie(t *testing.T) { + t.Parallel() + now := time.Now() + provider := &EntraProvider{ + config: EntraConfig{TenantID: "00000000-0000-4000-8000-000000000001"}, + flows: map[string]pendingEntraFlow{}, + now: func() time.Time { return now }, + } + provider.flows["state"] = pendingEntraFlow{CookieHash: sha256.Sum256([]byte("browser-cookie")), ExpiresAt: now.Add(time.Minute)} + if _, err := provider.Complete(t.Context(), "state", "attacker-cookie", "code"); err == nil { + t.Fatal("completion with the wrong flow cookie succeeded") + } + if _, exists := provider.flows["state"]; !exists { + t.Fatal("a caller without the flow cookie cancelled the pending login") + } +} diff --git a/internal/auth/manager.go b/internal/auth/manager.go index a152570..bbfb4b7 100644 --- a/internal/auth/manager.go +++ b/internal/auth/manager.go @@ -277,13 +277,13 @@ func (m *Manager) bootstrap(ctx context.Context, config Config) error { admin := &next.Users[adminIndex] current := "" if admin.EncryptedTOTPSecret != "" { - current, err = openSecret(m.encryptionKey, admin.EncryptedTOTPSecret) + current, err = openSecret(m.encryptionKey, admin.EncryptedTOTPSecret, admin.ID) if err != nil { return fmt.Errorf("opening bootstrap administrator totp secret: %w", err) } } if current != secret { - admin.EncryptedTOTPSecret, err = sealSecret(m.encryptionKey, secret) + admin.EncryptedTOTPSecret, err = sealSecret(m.encryptionKey, secret, admin.ID) if err != nil { return err } @@ -293,6 +293,20 @@ func (m *Manager) bootstrap(ctx context.Context, config Config) error { changed = true } } + for index := range next.Users { + user := &next.Users[index] + if user.EncryptedTOTPSecret == "" || secretIsBound(user.EncryptedTOTPSecret) { + continue + } + secret, err := openSecret(m.encryptionKey, user.EncryptedTOTPSecret, user.ID) + if err != nil { + return fmt.Errorf("opening legacy totp secret: %w", err) + } + if user.EncryptedTOTPSecret, err = sealSecret(m.encryptionKey, secret, user.ID); err != nil { + return err + } + changed = true + } if !changed { return nil } @@ -309,6 +323,14 @@ func (m *Manager) CookieSecure() bool { return m.cookieSecure } // addresses are honored for authentication throttling. func (m *Manager) TrustedProxies() []netip.Prefix { return slices.Clone(m.trustedProxies) } +// GuestOrganizationBinding returns the organization bound to the Guest +// workspace even while Guest access is disabled. +func (m *Manager) GuestOrganizationBinding() string { + m.mu.Lock() + defer m.mu.Unlock() + return m.guestOrganization +} + // GuestOrganizationID returns the stable organization used for newly created // Guest workspace topologies. It is empty when Guest access is disabled. func (m *Manager) GuestOrganizationID() string { @@ -430,7 +452,7 @@ func (m *Manager) CompleteSetup(ctx context.Context, challengeToken, code string if err != nil { return Session{}, nil, err } - encryptedSecret, err := sealSecret(m.encryptionKey, secret) + encryptedSecret, err := sealSecret(m.encryptionKey, secret, user.ID) if err != nil { return Session{}, nil, err } @@ -501,7 +523,7 @@ func (m *Manager) completeAuthenticator(ctx context.Context, challengeToken, cod m.mu.Unlock() return Session{}, err } - secret, err := openSecret(m.encryptionKey, user.EncryptedTOTPSecret) + secret, err := openSecret(m.encryptionKey, user.EncryptedTOTPSecret, user.ID) lastStep := user.LastTOTPStep m.mu.Unlock() if err != nil { @@ -1100,13 +1122,28 @@ func positiveOr(value, fallback int) int { return fallback } +// remoteHost returns the throttling key for a client address. IPv6 clients +// usually control a whole /64, so their addresses are aggregated to that +// prefix; otherwise one client could rotate through fresh per-address budgets. func remoteHost(value string) string { value = strings.TrimSpace(value) host, _, err := net.SplitHostPort(value) - if err == nil { - return strings.ToLower(host) + if err != nil { + host = value + } + host = strings.ToLower(host) + address, parseErr := netip.ParseAddr(host) + if parseErr != nil { + return host + } + address = address.Unmap() + if address.Is6() { + prefix, prefixErr := address.Prefix(64) + if prefixErr == nil { + return prefix.String() + } } - return strings.ToLower(value) + return address.String() } func validateUsername(value string) error { @@ -1289,7 +1326,7 @@ func validatePersistentState( return errors.New("auth: account state contains an invalid authentication source") } if user.EncryptedTOTPSecret != "" { - secret, err := openSecret(encryptionKey, user.EncryptedTOTPSecret) + secret, err := openSecret(encryptionKey, user.EncryptedTOTPSecret, user.ID) if err != nil { return err } diff --git a/internal/auth/manager_test.go b/internal/auth/manager_test.go index 92874c4..b5a1b48 100644 --- a/internal/auth/manager_test.go +++ b/internal/auth/manager_test.go @@ -3,6 +3,8 @@ package auth import ( "bytes" "context" + "crypto/aes" + "crypto/cipher" "encoding/base64" "errors" "fmt" @@ -323,24 +325,30 @@ func TestSecretEncryptionRoundTrip(t *testing.T) { for index := range key { key[index] = byte(index + 1) } - sealed, err := sealSecret(key, "JBSWY3DPEHPK3PXP") + sealed, err := sealSecret(key, "JBSWY3DPEHPK3PXP", "user-a") if err != nil { t.Fatal(err) } - opened, err := openSecret(key, sealed) + if !secretIsBound(sealed) { + t.Fatalf("sealed secret %q is not in the bound envelope format", sealed) + } + if _, err := openSecret(key, sealed, "user-b"); err == nil { + t.Fatal("secret sealed for one account opened for another account") + } + opened, err := openSecret(key, sealed, "user-a") if err != nil { t.Fatal(err) } if opened != "JBSWY3DPEHPK3PXP" { t.Fatalf("opened = %q", opened) } - sealedBytes, err := base64.RawStdEncoding.DecodeString(sealed) + sealedBytes, err := base64.RawStdEncoding.DecodeString(strings.TrimPrefix(sealed, boundSecretPrefix)) if err != nil { t.Fatal(err) } sealedBytes[len(sealedBytes)-1] ^= 1 - tampered := base64.RawStdEncoding.EncodeToString(sealedBytes) - if _, err := openSecret(key, tampered); err == nil { + tampered := boundSecretPrefix + base64.RawStdEncoding.EncodeToString(sealedBytes) + if _, err := openSecret(key, tampered, "user-a"); err == nil { t.Fatal("tampered ciphertext was accepted") } } @@ -826,3 +834,43 @@ func TestManagerValidationAndExpiryEdges(t *testing.T) { t.Fatalf("locked second factor error = %v", err) } } + +func TestLegacyUnboundSecretStillOpens(t *testing.T) { + t.Parallel() + key := make([]byte, 32) + block, err := aes.NewCipher(key) + if err != nil { + t.Fatal(err) + } + aead, err := cipher.NewGCM(block) + if err != nil { + t.Fatal(err) + } + nonce := make([]byte, aead.NonceSize()) + legacy := base64.RawStdEncoding.EncodeToString(aead.Seal(nonce, nonce, []byte("JBSWY3DPEHPK3PXP"), nil)) // #nosec G407 -- fixed nonce builds a legacy-format test fixture. + if secretIsBound(legacy) { + t.Fatal("legacy envelope reported as bound") + } + opened, err := openSecret(key, legacy, "any-user") + if err != nil || opened != "JBSWY3DPEHPK3PXP" { + t.Fatalf("legacy secret opened = %q, %v", opened, err) + } +} + +func TestRemoteHostAggregatesIPv6Prefixes(t *testing.T) { + t.Parallel() + first := remoteHost("[2001:db8:1:2:aaaa::1]:51000") + second := remoteHost("[2001:db8:1:2:bbbb::9]:443") + if first != second || first != "2001:db8:1:2::/64" { + t.Fatalf("IPv6 keys = %q and %q, want one shared /64 key", first, second) + } + if other := remoteHost("[2001:db8:1:3::1]:1"); other == first { + t.Fatalf("different /64 shares key %q", other) + } + if got := remoteHost("192.0.2.10:8080"); got != "192.0.2.10" { + t.Fatalf("IPv4 key = %q, want 192.0.2.10", got) + } + if got := remoteHost("[::ffff:192.0.2.10]:8080"); got != "192.0.2.10" { + t.Fatalf("IPv4-mapped key = %q, want 192.0.2.10", got) + } +} diff --git a/internal/auth/password.go b/internal/auth/password.go index fe50b3a..e37b6a7 100644 --- a/internal/auth/password.go +++ b/internal/auth/password.go @@ -6,6 +6,7 @@ import ( "encoding/base64" "errors" "fmt" + "runtime" "strconv" "strings" @@ -20,6 +21,17 @@ const ( argonSaltLen = 16 ) +// argonSlots bounds concurrent Argon2id computations process-wide. Each one +// allocates argonMemory KiB, so per-client throttles alone would let clients +// spread across many addresses hold an unbounded amount of hashing memory. +var argonSlots = make(chan struct{}, max(4, runtime.GOMAXPROCS(0)*2)) + +func argonIDKey(password, salt []byte, time, memory uint32, threads uint8, keyLength uint32) []byte { + argonSlots <- struct{}{} + defer func() { <-argonSlots }() + return argon2.IDKey(password, salt, time, memory, threads, keyLength) +} + func hashPassword(password string) (string, error) { if err := validatePassword(password); err != nil { return "", err @@ -28,7 +40,7 @@ func hashPassword(password string) (string, error) { if _, err := rand.Read(salt); err != nil { return "", fmt.Errorf("generating password salt: %w", err) } - hash := argon2.IDKey([]byte(password), salt, argonTime, argonMemory, argonThreads, argonKeyLen) + hash := argonIDKey([]byte(password), salt, argonTime, argonMemory, argonThreads, argonKeyLen) return fmt.Sprintf( "$argon2id$v=%d$m=%d,t=%d,p=%d$%s$%s", argon2.Version, @@ -47,7 +59,7 @@ func comparePassword(encoded, password string) (bool, error) { } // parsePasswordHash caps decoded hashes at 64 bytes before this conversion. keyLength := uint32(len(expected)) // #nosec G115 -- len(expected) is validated in the range 16..64. - actual := argon2.IDKey( + actual := argonIDKey( []byte(password), salt, parameters.time, parameters.memory, parameters.threads, keyLength, ) return subtle.ConstantTimeCompare(actual, expected) == 1, nil diff --git a/internal/auth/persistence.go b/internal/auth/persistence.go index a66e690..1298ec9 100644 --- a/internal/auth/persistence.go +++ b/internal/auth/persistence.go @@ -13,6 +13,7 @@ import ( "os" "path/filepath" "slices" + "strings" "time" ) @@ -223,7 +224,17 @@ func writePrivateFile(path string, data []byte) (returnErr error) { return os.Rename(temporaryName, path) } -func sealSecret(key []byte, value string) (string, error) { +// boundSecretPrefix marks envelopes whose GCM associated data binds the +// ciphertext to its owning account, so a database writer cannot move one +// user's sealed TOTP secret onto another account. Unprefixed envelopes are the +// legacy unbound format; they still open and are resealed at startup. +const boundSecretPrefix = "v2." + +func secretAssociatedData(owner string) []byte { + return []byte("wiredraft-totp:" + owner) +} + +func sealSecret(key []byte, value, owner string) (string, error) { block, err := aes.NewCipher(key) if err != nil { return "", fmt.Errorf("creating secret cipher: %w", err) @@ -236,11 +247,18 @@ func sealSecret(key []byte, value string) (string, error) { if _, err := rand.Read(nonce); err != nil { return "", fmt.Errorf("generating secret nonce: %w", err) } - sealed := aead.Seal(nonce, nonce, []byte(value), nil) - return base64.RawStdEncoding.EncodeToString(sealed), nil + sealed := aead.Seal(nonce, nonce, []byte(value), secretAssociatedData(owner)) + return boundSecretPrefix + base64.RawStdEncoding.EncodeToString(sealed), nil } -func openSecret(key []byte, encoded string) (string, error) { +func secretIsBound(encoded string) bool { return strings.HasPrefix(encoded, boundSecretPrefix) } + +func openSecret(key []byte, encoded, owner string) (string, error) { + var associatedData []byte + if secretIsBound(encoded) { + encoded = strings.TrimPrefix(encoded, boundSecretPrefix) + associatedData = secretAssociatedData(owner) + } sealed, err := base64.RawStdEncoding.DecodeString(encoded) if err != nil { return "", errors.New("auth: invalid encrypted secret") @@ -256,7 +274,7 @@ func openSecret(key []byte, encoded string) (string, error) { if len(sealed) < aead.NonceSize() { return "", errors.New("auth: encrypted secret is too short") } - plaintext, err := aead.Open(nil, sealed[:aead.NonceSize()], sealed[aead.NonceSize():], nil) + plaintext, err := aead.Open(nil, sealed[:aead.NonceSize()], sealed[aead.NonceSize():], associatedData) if err != nil { return "", errors.New("auth: decrypting secret failed") } diff --git a/internal/auth/postgres.go b/internal/auth/postgres.go index 15e3f19..39943c8 100644 --- a/internal/auth/postgres.go +++ b/internal/auth/postgres.go @@ -318,16 +318,17 @@ func applyExternalSecretKey( if sealed == "" { continue } - if _, err := openSecret(externalKey, sealed); err == nil { + owner := state.Users[index].ID + if _, err := openSecret(externalKey, sealed, owner); err == nil { continue } opened := false for _, candidate := range candidates { - secret, err := openSecret(candidate, sealed) + secret, err := openSecret(candidate, sealed, owner) if err != nil { continue } - resealed, err := sealSecret(externalKey, secret) + resealed, err := sealSecret(externalKey, secret, owner) if err != nil { return nil, false, err } diff --git a/internal/auth/secretkey_test.go b/internal/auth/secretkey_test.go index 608131e..3ccec8f 100644 --- a/internal/auth/secretkey_test.go +++ b/internal/auth/secretkey_test.go @@ -33,7 +33,7 @@ func TestApplyExternalSecretKeyMigratesAndRotates(t *testing.T) { externalKey := bytes.Repeat([]byte{2}, 32) rotatedKey := bytes.Repeat([]byte{3}, 32) const secret = "JBSWY3DPEHPK3PXP" // #nosec G101 -- public RFC-compatible test fixture. - sealed, err := sealSecret(storedKey, secret) + sealed, err := sealSecret(storedKey, secret, "user") if err != nil { t.Fatal(err) } @@ -49,10 +49,10 @@ func TestApplyExternalSecretKeyMigratesAndRotates(t *testing.T) { if err != nil || !changed || !bytes.Equal(key, externalKey) || !state.ExternalSecretKey { t.Fatalf("migration = %x, %v, %v", key, changed, err) } - if opened, err := openSecret(externalKey, state.Users[0].EncryptedTOTPSecret); err != nil || opened != secret { + if opened, err := openSecret(externalKey, state.Users[0].EncryptedTOTPSecret, "user"); err != nil || opened != secret { t.Fatalf("migrated secret = %q, %v", opened, err) } - if _, err := openSecret(storedKey, state.Users[0].EncryptedTOTPSecret); err == nil { + if _, err := openSecret(storedKey, state.Users[0].EncryptedTOTPSecret, "user"); err == nil { t.Fatal("migrated secret still opens with the database-stored key") } @@ -71,7 +71,7 @@ func TestApplyExternalSecretKeyMigratesAndRotates(t *testing.T) { if err != nil || !changed || !bytes.Equal(key, rotatedKey) { t.Fatalf("rotation = %x, %v, %v", key, changed, err) } - if opened, err := openSecret(rotatedKey, state.Users[0].EncryptedTOTPSecret); err != nil || opened != secret { + if opened, err := openSecret(rotatedKey, state.Users[0].EncryptedTOTPSecret, "user"); err != nil || opened != secret { t.Fatalf("rotated secret = %q, %v", opened, err) } } diff --git a/internal/handler/auth.go b/internal/handler/auth.go index f985e53..5bafdf0 100644 --- a/internal/handler/auth.go +++ b/internal/handler/auth.go @@ -103,7 +103,7 @@ func (s *Server) verifyTOTP(w http.ResponseWriter, request *http.Request) { s.secondFactorFailure(w, "totp", err) return } - s.setSessionCookie(w, session) + s.setSessionCookie(w, request, session) s.logger.Info("authentication succeeded", "user_id", session.Principal.UserID, "role", session.Principal.Role, "factor", "totp") s.writeSession(w, session, nil) } @@ -119,7 +119,7 @@ func (s *Server) completeTOTPSetup(w http.ResponseWriter, request *http.Request) s.secondFactorFailure(w, "totp_setup", err) return } - s.setSessionCookie(w, session) + s.setSessionCookie(w, request, session) s.logger.Info("totp enrollment completed", "user_id", session.Principal.UserID, "role", session.Principal.Role) s.writeSession(w, session, recoveryCodes) } @@ -135,7 +135,7 @@ func (s *Server) verifyRecoveryCode(w http.ResponseWriter, request *http.Request s.secondFactorFailure(w, "recovery_code", err) return } - s.setSessionCookie(w, session) + s.setSessionCookie(w, request, session) s.logger.Warn("recovery code used", "user_id", session.Principal.UserID, "role", session.Principal.Role) s.writeSession(w, session, nil) } @@ -158,7 +158,7 @@ func (s *Server) guestLogin(w http.ResponseWriter, request *http.Request) { s.authFailure(w, err) return } - s.setSessionCookie(w, session) + s.setSessionCookie(w, request, session) s.writeSession(w, session, nil) } @@ -198,7 +198,7 @@ func (s *Server) entraCallback(w http.ResponseWriter, request *http.Request) { return } s.clearEntraFlowCookie(w) - s.setSessionCookie(w, session) + s.setSessionCookie(w, request, session) s.logger.Info("authentication succeeded", "user_id", session.Principal.UserID, "role", session.Principal.Role, "provider", "entra") http.Redirect(w, request, "/", http.StatusSeeOther) } @@ -247,6 +247,12 @@ func (s *Server) listUsers(w http.ResponseWriter, request *http.Request) { } func (s *Server) createUser(w http.ResponseWriter, request *http.Request) { + // The body is read before the directory lock so a slow client cannot hold it. + var input createUserRequest + if err := decodeJSON(w, request, &input); err != nil { + writeError(w, http.StatusBadRequest, "invalid account request") + return + } s.directoryMu.Lock() defer s.directoryMu.Unlock() administrator, authorized := s.currentAdministrator(w, request) @@ -254,11 +260,6 @@ func (s *Server) createUser(w http.ResponseWriter, request *http.Request) { return } - var input createUserRequest - if err := decodeJSON(w, request, &input); err != nil { - writeError(w, http.StatusBadRequest, "invalid account request") - return - } access, err := s.canonicalAccess(request.Context(), input.Access) if err != nil { s.authFailure(w, err) @@ -297,6 +298,12 @@ func (s *Server) createUser(w http.ResponseWriter, request *http.Request) { } func (s *Server) updateUser(w http.ResponseWriter, request *http.Request) { + // The body is read before the directory lock so a slow client cannot hold it. + var input updateUserRequest + if err := decodeJSON(w, request, &input); err != nil { + writeError(w, http.StatusBadRequest, "invalid account update") + return + } s.directoryMu.Lock() defer s.directoryMu.Unlock() administrator, authorized := s.currentAdministrator(w, request) @@ -304,11 +311,6 @@ func (s *Server) updateUser(w http.ResponseWriter, request *http.Request) { return } - var input updateUserRequest - if err := decodeJSON(w, request, &input); err != nil { - writeError(w, http.StatusBadRequest, "invalid account update") - return - } access, err := s.canonicalUpdatedAccess( request.Context(), request.PathValue("userId"), input.Access, input.Disabled, input.ResetExternalIdentity, @@ -456,7 +458,12 @@ func (s *Server) sessionFromRequest(request *http.Request) (auth.Session, bool) return s.auth.Session(cookie.Value) } -func (s *Server) setSessionCookie(w http.ResponseWriter, session auth.Session) { +func (s *Server) setSessionCookie(w http.ResponseWriter, request *http.Request, session auth.Session) { + // A new sign-in replaces the browser's session, so the session the cookie + // previously named is revoked instead of lingering until it expires. + if previous, err := request.Cookie(sessionCookieName); err == nil && previous.Value != "" && previous.Value != session.Token { + s.auth.Logout(previous.Value) + } // Secure is environment-controlled so local HTTP remains usable; production // TLS deployments must enable WIREDRAFT_COOKIE_SECURE. HttpOnly and strict // same-site protections are unconditional. @@ -599,6 +606,13 @@ func (s *Server) clientAddress(request *http.Request) string { proxies := s.auth.TrustedProxies() peer, ok := parseClientIP(request.RemoteAddr) if len(proxies) == 0 || !ok || !containsAddr(proxies, peer) { + if len(proxies) == 0 && request.Header.Get("X-Forwarded-For") != "" { + // Behind an unconfigured proxy every client shares the proxy's + // address, so one client can exhaust everyone's login budget. + s.forwardedWarning.Do(func() { + s.logger.Warn("request carries X-Forwarded-For but WIREDRAFT_TRUSTED_PROXIES is empty; throttling uses the proxy address for every client") + }) + } return request.RemoteAddr } var hops []string diff --git a/internal/handler/auth_test.go b/internal/handler/auth_test.go index f507fae..e6064c4 100644 --- a/internal/handler/auth_test.go +++ b/internal/handler/auth_test.go @@ -747,6 +747,9 @@ func newJSONRequest(t *testing.T, method, path string, body any, cookie *http.Co if body != nil { request.Header.Set("Content-Type", "application/json") } + if topology, replacing := body.(model.Topology); replacing && method == http.MethodPut { + request.Header.Set("If-Match", topologyRevisionETag(topology.Revision)) + } if cookie != nil { request.AddCookie(cookie) } diff --git a/internal/handler/collaboration_test.go b/internal/handler/collaboration_test.go index a0a9f6d..82c96a1 100644 --- a/internal/handler/collaboration_test.go +++ b/internal/handler/collaboration_test.go @@ -2,6 +2,8 @@ package handler import ( "bytes" + "crypto/sha256" + "encoding/hex" "encoding/json" "fmt" "net/http" @@ -296,33 +298,41 @@ func TestReplaceTopologyCannotResurrectRevokedShare(t *testing.T) { if err := json.Unmarshal(response.Body.Bytes(), &share); err != nil { t.Fatal(err) } - // A client snapshot taken while the grant existed, as kept by undo history - // or a JSON backup. - stale := requestTopology(t, handler, http.MethodGet, "/api/v1/topologies/"+topology.ID, nil, http.StatusOK) - if len(stale.ShareGrants) != 1 { - t.Fatalf("share grants before revocation = %d, want 1", len(stale.ShareGrants)) + if share.ExpiresAt == nil { + t.Fatal("share was created without an expiry") + } + current := requestTopology(t, handler, http.MethodGet, "/api/v1/topologies/"+topology.ID, nil, http.StatusOK) + if len(current.ShareGrants) != 0 { + t.Fatalf("topology response exposes %d share grants", len(current.ShareGrants)) } response = httptest.NewRecorder() - handler.ServeHTTP(response, httptest.NewRequestWithContext(t.Context(), http.MethodDelete, sharesPath+"/"+stale.ShareGrants[0].ID, nil)) + handler.ServeHTTP(response, httptest.NewRequestWithContext(t.Context(), http.MethodDelete, sharesPath+"/"+share.ID, nil)) if response.Code != http.StatusNoContent { t.Fatalf("delete share status = %d, want 204", response.Code) } - replaced := requestTopology(t, handler, http.MethodPut, "/api/v1/topologies/"+topology.ID, stale, http.StatusOK) - if len(replaced.ShareGrants) != 0 { - t.Fatalf("share grants after replace = %d, want 0", len(replaced.ShareGrants)) - } + // A client that kept the grant (undo history, a JSON backup) replays it. + digest := sha256.Sum256([]byte(share.Token)) + current = requestTopology(t, handler, http.MethodGet, "/api/v1/topologies/"+topology.ID, nil, http.StatusOK) + forged := current + forged.ShareGrants = []model.ShareGrant{{ + ID: share.ID, Name: share.Name, TokenHash: hex.EncodeToString(digest[:]), CreatedAt: share.CreatedAt, + }} + requestTopology(t, handler, http.MethodPut, "/api/v1/topologies/"+topology.ID, forged, http.StatusOK) + response = httptest.NewRecorder() handler.ServeHTTP(response, httptest.NewRequestWithContext(t.Context(), http.MethodGet, share.Path, nil)) if response.Code != http.StatusNotFound { t.Fatalf("revoked share read after replace status = %d, want 404", response.Code) } - - forged := replaced - forged.ShareGrants = stale.ShareGrants - replaced = requestTopology(t, handler, http.MethodPut, "/api/v1/topologies/"+topology.ID, forged, http.StatusOK) - if len(replaced.ShareGrants) != 0 { - t.Fatalf("client-supplied share grants were persisted: %#v", replaced.ShareGrants) + response = httptest.NewRecorder() + handler.ServeHTTP(response, httptest.NewRequestWithContext(t.Context(), http.MethodGet, sharesPath, nil)) + var shares []shareGrantResponse + if err := json.Unmarshal(response.Body.Bytes(), &shares); err != nil { + t.Fatal(err) + } + if len(shares) != 0 { + t.Fatalf("client-supplied share grants were persisted: %#v", shares) } } diff --git a/internal/handler/duplicate.go b/internal/handler/duplicate.go index 5d17ae7..631d5b8 100644 --- a/internal/handler/duplicate.go +++ b/internal/handler/duplicate.go @@ -25,8 +25,8 @@ func (s *Server) duplicateTopology(w http.ResponseWriter, request *http.Request) writeError(w, http.StatusBadRequest, "the source revision is required") return } - s.directoryMu.Lock() - defer s.directoryMu.Unlock() + s.directoryMu.RLock() + defer s.directoryMu.RUnlock() source, err := s.getAuthorizedTopology(request, request.PathValue("id")) if err != nil { s.fail(w, err) @@ -39,6 +39,11 @@ func (s *Server) duplicateTopology(w http.ResponseWriter, request *http.Request) return } } + s.createMu.Lock() + defer s.createMu.Unlock() + if !s.organizationHasCapacity(w, request, source.OrganizationID) { + return + } if source.Revision != expected { s.fail(w, &store.RevisionConflictError{Expected: expected, Actual: source.Revision}) return diff --git a/internal/handler/hardening_test.go b/internal/handler/hardening_test.go new file mode 100644 index 0000000..9e7a99b --- /dev/null +++ b/internal/handler/hardening_test.go @@ -0,0 +1,201 @@ +package handler + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + "wiredraft/internal/auth" + "wiredraft/internal/model" +) + +func TestReplaceTopologyRequiresIfMatch(t *testing.T) { + t.Parallel() + handler := newTestHandler(t) + topology := requestTopology(t, handler, http.MethodPost, "/api/v1/topologies", map[string]string{ + "name": "Revisioned", "template": "blank", + }, http.StatusCreated) + body, err := json.Marshal(topology) + if err != nil { + t.Fatal(err) + } + request := httptest.NewRequestWithContext(t.Context(), http.MethodPut, "/api/v1/topologies/"+topology.ID, bytes.NewReader(body)) + request.Header.Set("Content-Type", "application/json") + response := httptest.NewRecorder() + handler.ServeHTTP(response, request) + if response.Code != http.StatusPreconditionRequired { + t.Fatalf("replace without If-Match status = %d, want 428", response.Code) + } +} + +func TestShareGrantsAlwaysExpire(t *testing.T) { + t.Parallel() + handler := newTestHandler(t) + topology := requestTopology(t, handler, http.MethodPost, "/api/v1/topologies", map[string]string{ + "name": "Expiring shares", "template": "blank", + }, http.StatusCreated) + sharesPath := "/api/v1/topologies/" + topology.ID + "/shares" + createShare := func(input createShareGrantRequest) *httptest.ResponseRecorder { + body, err := json.Marshal(input) + if err != nil { + t.Fatal(err) + } + request := httptest.NewRequestWithContext(t.Context(), http.MethodPost, sharesPath, bytes.NewReader(body)) + request.Header.Set("Content-Type", "application/json") + response := httptest.NewRecorder() + handler.ServeHTTP(response, request) + return response + } + + response := createShare(createShareGrantRequest{Name: "Default"}) + if response.Code != http.StatusCreated { + t.Fatalf("create share status = %d; body = %s", response.Code, response.Body.String()) + } + var share shareGrantResponse + if err := json.Unmarshal(response.Body.Bytes(), &share); err != nil { + t.Fatal(err) + } + if share.ExpiresAt == nil || time.Until(*share.ExpiresAt) > defaultShareLifetime || time.Until(*share.ExpiresAt) < defaultShareLifetime-time.Hour { + t.Fatalf("default share expiry = %v, want about %v from now", share.ExpiresAt, defaultShareLifetime) + } + + tooLate := time.Now().Add(maxShareLifetime + 24*time.Hour) + if response := createShare(createShareGrantRequest{Name: "Forever", ExpiresAt: &tooLate}); response.Code != http.StatusBadRequest { + t.Fatalf("share beyond the maximum lifetime status = %d, want 400", response.Code) + } + past := time.Now().Add(-time.Hour) + if response := createShare(createShareGrantRequest{Name: "Expired", ExpiresAt: &past}); response.Code != http.StatusBadRequest { + t.Fatalf("already-expired share status = %d, want 400", response.Code) + } +} + +func TestGuestSessionAttributionAndSessionReplacement(t *testing.T) { + t.Parallel() + handler, _, _ := newAuthenticatedTestHandler(t, auth.Config{ + AdminUsername: "operator", AdminPassword: authTestPassword, GuestEnabled: true, + }) + guestLogin := func(cookie *http.Cookie) (*http.Cookie, string) { + request := newJSONRequest(t, http.MethodPost, "/api/v1/auth/guest", map[string]string{}, cookie) + request.Header.Set("Origin", "http://example.com") + response := httptest.NewRecorder() + handler.ServeHTTP(response, request) + if response.Code != http.StatusOK { + t.Fatalf("guest login status = %d; body = %s", response.Code, response.Body.String()) + } + var session struct { + Session auth.SessionView `json:"session"` + } + decodeResponse(t, response, &session) + return response.Result().Cookies()[0], session.Session.CSRFToken + } + first, _ := guestLogin(nil) + second, csrf := guestLogin(first) + status := performJSONRequest(t, handler, http.MethodGet, "/api/v1/auth/status", nil, first) + var view struct { + Session auth.SessionView `json:"session"` + } + decodeResponse(t, status, &view) + if view.Session.Authenticated { + t.Fatal("the session replaced by a new sign-in is still valid") + } + + send := func(method, path string, body any) *httptest.ResponseRecorder { + request := newJSONRequest(t, method, path, body, second) + request.Header.Set("Origin", "http://example.com") + request.Header.Set("X-CSRF-Token", csrf) + response := httptest.NewRecorder() + handler.ServeHTTP(response, request) + return response + } + created := send(http.MethodPost, "/api/v1/topologies", map[string]string{"name": "Guest map", "template": "demo"}) + if created.Code != http.StatusCreated { + t.Fatalf("guest create status = %d; body = %s", created.Code, created.Body.String()) + } + var topology model.Topology + decodeResponse(t, created, &topology) + commented := send(http.MethodPost, "/api/v1/topologies/"+topology.ID+"/comments", createCommentThreadRequest{ + Anchor: model.CommentAnchor{Kind: model.CommentAnchorDevice, TargetID: topology.Devices[0].ID}, + Author: "Network Admin", Body: "Spoofed attribution", + }) + if commented.Code != http.StatusCreated { + t.Fatalf("comment status = %d; body = %s", commented.Code, commented.Body.String()) + } + decodeResponse(t, commented, &topology) + if author := topology.CommentThreads[0].Messages[0].Author; author != "Guest" { + t.Fatalf("comment author = %q, want the session principal", author) + } +} + +func TestGuestOrganizationNameIsReserved(t *testing.T) { + t.Parallel() + handler, authManager, _ := newAuthenticatedTestHandler(t, auth.Config{ + AdminUsername: "admin", AdminPassword: authTestPassword, + }) + adminSession := authenticateTestUser(t, authManager, "admin", authTestPassword) + response := performSessionJSONRequest(t, handler, adminSession, + http.MethodPost, "/api/v1/admin/organizations", organizationMutationRequest{Name: " guest "}) + if response.Code != http.StatusConflict { + t.Fatalf("create organization named guest status = %d, want 409", response.Code) + } + member := createTestOrganizationThroughAPI(t, handler, adminSession, "Branch") + response = performSessionJSONRequest(t, handler, adminSession, + http.MethodPut, "/api/v1/admin/organizations/"+member.ID, organizationMutationRequest{Name: "GUEST"}) + if response.Code != http.StatusConflict { + t.Fatalf("rename organization to GUEST status = %d, want 409", response.Code) + } +} + +func TestOrganizationMoveRevokesShareGrants(t *testing.T) { + t.Parallel() + handler, authManager, _ := newAuthenticatedTestHandler(t, auth.Config{ + AdminUsername: "admin", AdminPassword: authTestPassword, + }) + adminSession := authenticateTestUser(t, authManager, "admin", authTestPassword) + target := createTestOrganizationThroughAPI(t, handler, adminSession, "Restricted") + created := performSessionJSONRequest(t, handler, adminSession, + http.MethodPost, "/api/v1/topologies", map[string]string{"name": "Moving map", "template": "blank"}) + if created.Code != http.StatusCreated { + t.Fatalf("create topology status = %d; body = %s", created.Code, created.Body.String()) + } + var topology model.Topology + decodeResponse(t, created, &topology) + sharesPath := "/api/v1/topologies/" + topology.ID + "/shares" + if response := performSessionJSONRequest(t, handler, adminSession, http.MethodPost, sharesPath, + createShareGrantRequest{Name: "Vendor"}); response.Code != http.StatusCreated { + t.Fatalf("create share status = %d; body = %s", response.Code, response.Body.String()) + } + current := performSessionJSONRequest(t, handler, adminSession, http.MethodGet, "/api/v1/topologies/"+topology.ID, nil) + decodeResponse(t, current, &topology) + topology.OrganizationID = target.ID + topology.Organization = target.Name + moved := performSessionJSONRequest(t, handler, adminSession, http.MethodPut, "/api/v1/topologies/"+topology.ID, topology) + if moved.Code != http.StatusOK { + t.Fatalf("move status = %d; body = %s", moved.Code, moved.Body.String()) + } + listed := performSessionJSONRequest(t, handler, adminSession, http.MethodGet, sharesPath, nil) + var shares []shareGrantResponse + decodeResponse(t, listed, &shares) + if len(shares) != 0 { + t.Fatalf("share grants after organization move = %d, want 0", len(shares)) + } +} + +func TestLegacyShareGrantsWithoutExpiryStopWorking(t *testing.T) { + t.Parallel() + now := time.Now() + recent := model.ShareGrant{CreatedAt: now.Add(-24 * time.Hour)} + if !shareExpiry(recent).After(now) { + t.Fatal("recent legacy grant already expired") + } + old := model.ShareGrant{CreatedAt: now.Add(-maxShareLifetime - time.Hour)} + if shareExpiry(old).After(now) { + t.Fatal("legacy grant without expiry is still valid after the maximum lifetime") + } + explicit := now.Add(time.Hour) + if got := shareExpiry(model.ShareGrant{CreatedAt: now.Add(-maxShareLifetime * 2), ExpiresAt: &explicit}); !got.Equal(explicit) { + t.Fatalf("explicit expiry = %v, want %v", got, explicit) + } +} diff --git a/internal/handler/json.go b/internal/handler/json.go index fd28930..1bf3e38 100644 --- a/internal/handler/json.go +++ b/internal/handler/json.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "net/http" + "wiredraft/internal/model" ) const maxRequestBody = 2 << 20 @@ -32,6 +33,9 @@ func decodeJSON(w http.ResponseWriter, request *http.Request, destination any) e } func writeJSON(w http.ResponseWriter, status int, value any) { + if topology, ok := value.(model.Topology); ok { + value = publicTopology(topology) + } w.Header().Set("Content-Type", "application/json; charset=utf-8") w.WriteHeader(status) if err := json.NewEncoder(w).Encode(value); err != nil { @@ -39,6 +43,14 @@ func writeJSON(w http.ResponseWriter, status int, value any) { } } +// publicTopology removes share-grant token digests from a topology before it +// leaves the server. Clients list shares through the shares endpoint, so the +// digests never need to reach browsers, event streams or exported files. +func publicTopology(topology model.Topology) model.Topology { + topology.ShareGrants = nil + return topology +} + func writeError(w http.ResponseWriter, status int, message string) { writeJSON(w, status, errorResponse{Error: message, Code: status}) } diff --git a/internal/handler/organizations.go b/internal/handler/organizations.go index 690370f..1887255 100644 --- a/internal/handler/organizations.go +++ b/internal/handler/organizations.go @@ -33,6 +33,12 @@ func (s *Server) listOrganizations(w http.ResponseWriter, request *http.Request) } func (s *Server) createOrganization(w http.ResponseWriter, request *http.Request) { + // The body is read before the directory lock so a slow client cannot hold it. + var input organizationMutationRequest + if err := decodeJSON(w, request, &input); err != nil { + writeError(w, http.StatusBadRequest, "invalid organization request") + return + } s.directoryMu.Lock() defer s.directoryMu.Unlock() administrator, authorized := s.currentAdministrator(w, request) @@ -40,9 +46,8 @@ func (s *Server) createOrganization(w http.ResponseWriter, request *http.Request return } - var input organizationMutationRequest - if err := decodeJSON(w, request, &input); err != nil { - writeError(w, http.StatusBadRequest, "invalid organization request") + if isReservedGuestName(input.Name) { + writeError(w, http.StatusConflict, "Guest is reserved for the Guest workspace organization") return } organization, err := s.store.CreateOrganization(request.Context(), input.Name) @@ -68,6 +73,12 @@ func (s *Server) createOrganization(w http.ResponseWriter, request *http.Request } func (s *Server) updateOrganization(w http.ResponseWriter, request *http.Request) { + // The body is read before the directory lock so a slow client cannot hold it. + var input organizationMutationRequest + if err := decodeJSON(w, request, &input); err != nil { + writeError(w, http.StatusBadRequest, "invalid organization request") + return + } s.directoryMu.Lock() defer s.directoryMu.Unlock() administrator, authorized := s.currentAdministrator(w, request) @@ -79,9 +90,8 @@ func (s *Server) updateOrganization(w http.ResponseWriter, request *http.Request writeError(w, http.StatusForbidden, "disable Guest access before renaming its organization") return } - var input organizationMutationRequest - if err := decodeJSON(w, request, &input); err != nil { - writeError(w, http.StatusBadRequest, "invalid organization request") + if isReservedGuestName(input.Name) && request.PathValue("organizationId") != s.auth.GuestOrganizationBinding() { + writeError(w, http.StatusConflict, "Guest is reserved for the Guest workspace organization") return } organization, err := s.store.RenameOrganization( @@ -228,3 +238,10 @@ func (s *Server) resolveOrganization( } return organization, nil } + +// isReservedGuestName reports whether a name would be adopted as the anonymous +// Guest workspace at startup. Only the organization already bound to Guest may +// carry it, so no member organization can become anonymously accessible. +func isReservedGuestName(name string) bool { + return strings.EqualFold(strings.TrimSpace(name), auth.GuestOrganizationName) +} diff --git a/internal/handler/photos.go b/internal/handler/photos.go index 45f9de1..b48ed10 100644 --- a/internal/handler/photos.go +++ b/internal/handler/photos.go @@ -211,7 +211,7 @@ func (s *Server) getPhoto(w http.ResponseWriter, request *http.Request) { } w.Header().Set("Content-Type", photo.MediaType) w.Header().Set("Content-Disposition", mime.FormatMediaType("inline", map[string]string{"filename": photo.OriginalName})) - w.Header().Set("Cache-Control", "private, max-age=300") + w.Header().Set("Cache-Control", "private, no-cache") http.ServeContent(w, request, photo.OriginalName, info.ModTime(), file) } diff --git a/internal/handler/photos_test.go b/internal/handler/photos_test.go index beaa5d7..276a560 100644 --- a/internal/handler/photos_test.go +++ b/internal/handler/photos_test.go @@ -50,8 +50,8 @@ func TestPhotoUploadReadEditAndDelete(t *testing.T) { if response.Code != http.StatusOK || response.Header().Get("Content-Type") != "image/png" { t.Fatalf("photo response = %d %q, want 200 image/png", response.Code, response.Header().Get("Content-Type")) } - if response.Header().Get("Cache-Control") != "private, max-age=300" { - t.Fatalf("Cache-Control = %q, want private media cache", response.Header().Get("Cache-Control")) + if response.Header().Get("Cache-Control") != "private, no-cache" { + t.Fatalf("Cache-Control = %q, want private revalidated media cache", response.Header().Get("Cache-Control")) } topology = requestTopology(t, handler, http.MethodPut, diff --git a/internal/handler/server.go b/internal/handler/server.go index e566d8a..f0c9b57 100644 --- a/internal/handler/server.go +++ b/internal/handler/server.go @@ -30,22 +30,47 @@ import ( // Server coordinates HTTP handlers with persistence and event delivery. type Server struct { - store store.Store - broker *sse.Broker - logger *slog.Logger - static fs.FS - auth *auth.Manager - entra entraAuthenticator - media *media.Store - directoryMu sync.Mutex + store store.Store + broker *sse.Broker + logger *slog.Logger + static fs.FS + auth *auth.Manager + entra entraAuthenticator + media *media.Store + // directoryMu orders topology writes against directory changes (accounts, + // grants, organizations). Topology writes share it, because per-topology + // consistency comes from the store's row lock, so writes in different + // organizations never wait on each other; directory changes take it + // exclusively. + directoryMu sync.RWMutex streamMu sync.Mutex sessionStreams map[string]int + + forwardedWarning sync.Once + createMu sync.Mutex } // maxSessionEventStreams bounds concurrent event streams per login session. const maxSessionEventStreams = 8 +// computationDeadline bounds analysis-style requests. The server sets no global +// WriteTimeout because event streams are long-lived, so these routes get their +// own limit. +const computationDeadline = 30 * time.Second + +func withComputationDeadline(next http.HandlerFunc) http.HandlerFunc { + limited := http.TimeoutHandler(next, computationDeadline, `{"error":"analysis timed out","code":503}`) + return limited.ServeHTTP +} + +// Share links always expire: defaultShareLifetime applies when the client sets +// no expiry and maxShareLifetime caps any requested expiry. +const ( + defaultShareLifetime = 90 * 24 * time.Hour + maxShareLifetime = 365 * 24 * time.Hour +) + type entraAuthenticator interface { Begin(context.Context, string) (auth.EntraStart, error) Complete(context.Context, string, string, string) (auth.ExternalIdentity, error) @@ -183,8 +208,8 @@ func newHandler( protected("POST /api/v1/topologies/{id}/vlans", server.createVLAN) protected("PUT /api/v1/topologies/{id}/vlans/{vlanId}", server.updateVLAN) protected("DELETE /api/v1/topologies/{id}/vlans/{vlanId}", server.deleteVLAN) - protected("GET /api/v1/topologies/{id}/analysis", server.analysis) - protected("GET /api/v1/topologies/{id}/trace", server.trace) + protected("GET /api/v1/topologies/{id}/analysis", withComputationDeadline(server.analysis)) + protected("GET /api/v1/topologies/{id}/trace", withComputationDeadline(server.trace)) protected("GET /api/v1/topologies/{id}/events", server.events) protected("GET /api/v1/topologies/{id}/comments", server.listCommentThreads) protected("POST /api/v1/topologies/{id}/comments", server.createCommentThread) @@ -261,8 +286,8 @@ func (s *Server) createTopology(w http.ResponseWriter, request *http.Request) { topology.Location = strings.TrimSpace(input.Location) principal := principalFromRequest(request) if s.auth != nil { - s.directoryMu.Lock() - defer s.directoryMu.Unlock() + s.directoryMu.RLock() + defer s.directoryMu.RUnlock() session, authenticated := s.sessionFromRequest(request) if !authenticated { writeError(w, http.StatusUnauthorized, "authentication required") @@ -299,6 +324,13 @@ func (s *Server) createTopology(w http.ResponseWriter, request *http.Request) { return } } + // createMu makes the capacity count and the insert one step, so concurrent + // creates cannot all observe a count below the cap. + s.createMu.Lock() + defer s.createMu.Unlock() + if !s.organizationHasCapacity(w, request, topology.OrganizationID) { + return + } created, err := s.store.Create(request.Context(), topology) if err != nil { s.fail(w, err) @@ -307,6 +339,39 @@ func (s *Server) createTopology(w http.ResponseWriter, request *http.Request) { writeJSON(w, http.StatusCreated, created) } +// shareExpiry returns when a grant stops working. Grants created before expiry +// became mandatory have none stored; they expire maxShareLifetime after +// creation instead of staying valid indefinitely. +func shareExpiry(share model.ShareGrant) time.Time { + if share.ExpiresAt != nil { + return *share.ExpiresAt + } + return share.CreatedAt.Add(maxShareLifetime) +} + +// maxOrganizationTopologies bounds how many maps, and so how much attached +// media, one organization can accumulate on the shared server. +const maxOrganizationTopologies = 1000 + +func (s *Server) organizationHasCapacity(w http.ResponseWriter, request *http.Request, organizationID string) bool { + summaries, err := s.store.List(request.Context()) + if err != nil { + s.fail(w, err) + return false + } + count := 0 + for _, summary := range summaries { + if summary.OrganizationID == organizationID { + count++ + } + } + if count >= maxOrganizationTopologies { + writeError(w, http.StatusConflict, fmt.Sprintf("organization may contain at most %d maps", maxOrganizationTopologies)) + return false + } + return true +} + func (s *Server) getTopology(w http.ResponseWriter, request *http.Request) { topology, err := s.getAuthorizedTopology(request, request.PathValue("id")) if err != nil { @@ -329,6 +394,12 @@ func (s *Server) getAuthorizedTopology(request *http.Request, id string) (model. } func (s *Server) replaceTopology(w http.ResponseWriter, request *http.Request) { + // A whole-document replace must name the revision it was built from, so a + // stale client copy can never silently overwrite newer server state. + if strings.TrimSpace(request.Header.Get("If-Match")) == "" { + writeError(w, http.StatusPreconditionRequired, "If-Match revision is required") + return + } var input model.Topology if err := decodeJSON(w, request, &input); err != nil { writeError(w, http.StatusBadRequest, "invalid topology") @@ -369,10 +440,19 @@ func (s *Server) replaceTopology(w http.ResponseWriter, request *http.Request) { // endpoints mint or revoke them. Keeping the stored set stops undo, // backup import or any client body from resurrecting a revoked token. shareGrants := current.ShareGrants + // Comment threads are likewise managed only by the comment endpoints, + // which bind authorship to the session. + commentThreads := current.CommentThreads *current = input current.CreatedAt = createdAt current.Photos = photos current.ShareGrants = shareGrants + current.CommentThreads = commentThreads + if requestedOrganization != nil && requestedOrganization.ID != organizationID { + // Anonymous links issued under the previous organization must not + // keep exposing the map after it moves. + current.ShareGrants = nil + } if requestedOrganization == nil { current.OrganizationID = organizationID current.Organization = organization @@ -401,8 +481,8 @@ func (s *Server) deleteTopology(w http.ResponseWriter, request *http.Request) { s.fail(w, fmt.Errorf("%w: %w", store.ErrInvalid, err)) return } - s.directoryMu.Lock() - defer s.directoryMu.Unlock() + s.directoryMu.RLock() + defer s.directoryMu.RUnlock() if err := s.store.DeleteAtRevision(request.Context(), id, expectedRevision, func(topology model.Topology) error { if !s.authorizedTopologySnapshot(request, topology) { return store.ErrNotFound @@ -1336,7 +1416,7 @@ func (s *Server) createCommentThread(w http.ResponseWriter, request *http.Reques thread := model.CommentThread{ ID: threadID, Anchor: input.Anchor, CreatedAt: now, UpdatedAt: now, Messages: []model.CommentMessage{{ - ID: messageID, Author: strings.TrimSpace(input.Author), Body: strings.TrimSpace(input.Body), CreatedAt: now, UpdatedAt: now, + ID: messageID, Author: s.commentAuthor(request, input.Author), Body: strings.TrimSpace(input.Body), CreatedAt: now, UpdatedAt: now, }}, } topologyID := request.PathValue("id") @@ -1364,7 +1444,7 @@ func (s *Server) createCommentReply(w http.ResponseWriter, request *http.Request return } now := time.Now().UTC() - message := model.CommentMessage{ID: messageID, Author: strings.TrimSpace(input.Author), Body: strings.TrimSpace(input.Body), CreatedAt: now, UpdatedAt: now} + message := model.CommentMessage{ID: messageID, Author: s.commentAuthor(request, input.Author), Body: strings.TrimSpace(input.Body), CreatedAt: now, UpdatedAt: now} topologyID := request.PathValue("id") threadID := request.PathValue("threadId") updated, err := s.mutate(request, topologyID, func(topology *model.Topology) error { @@ -1513,7 +1593,8 @@ func (s *Server) listShareGrants(w http.ResponseWriter, request *http.Request) { } shares := make([]shareGrantResponse, 0, len(topology.ShareGrants)) for _, share := range topology.ShareGrants { - shares = append(shares, shareGrantResponse{ID: share.ID, Name: share.Name, CreatedAt: share.CreatedAt, ExpiresAt: share.ExpiresAt}) + expiresAt := shareExpiry(share) + shares = append(shares, shareGrantResponse{ID: share.ID, Name: share.Name, CreatedAt: share.CreatedAt, ExpiresAt: &expiresAt}) } writeJSON(w, http.StatusOK, shares) } @@ -1537,8 +1618,16 @@ func (s *Server) createShareGrant(w http.ResponseWriter, request *http.Request) token := base64.RawURLEncoding.EncodeToString(tokenBytes) digest := sha256.Sum256([]byte(token)) now := time.Now().UTC() + expiresAt := now.Add(defaultShareLifetime) + if input.ExpiresAt != nil { + expiresAt = input.ExpiresAt.UTC() + } + if !expiresAt.After(now) || expiresAt.After(now.Add(maxShareLifetime)) { + writeError(w, http.StatusBadRequest, "share expiry must be in the future and at most 365 days away") + return + } grant := model.ShareGrant{ - ID: shareID, Name: strings.TrimSpace(input.Name), TokenHash: hex.EncodeToString(digest[:]), CreatedAt: now, ExpiresAt: input.ExpiresAt, + ID: shareID, Name: strings.TrimSpace(input.Name), TokenHash: hex.EncodeToString(digest[:]), CreatedAt: now, ExpiresAt: &expiresAt, } if grant.Name == "" { grant.Name = "Read-only share" @@ -1599,7 +1688,7 @@ func (s *Server) getSharedTopology(w http.ResponseWriter, request *http.Request) for _, share := range topology.ShareGrants { expected, decodeErr := hex.DecodeString(share.TokenHash) matches := decodeErr == nil && len(expected) == len(digest) && subtle.ConstantTimeCompare(expected, digest[:]) == 1 - if matches && (share.ExpiresAt == nil || share.ExpiresAt.After(now)) { + if matches && shareExpiry(share).After(now) { granted = true } } @@ -1651,14 +1740,19 @@ func (s *Server) events(w http.ResponseWriter, request *http.Request) { if !open { return } - if !s.eventStreamAuthorized(request, topologyID) { + organizationID, authorized := s.eventStreamAuthorized(request, topologyID) + if !authorized { return } + if event.OrganizationID != "" && event.OrganizationID != organizationID { + // Produced while the topology belonged to another organization. + continue + } if _, err := fmt.Fprintf(w, "event: %s\ndata: %s\n\n", event.Type, event.Data); err != nil { return } case <-heartbeat.C: - if !s.eventStreamAuthorized(request, topologyID) { + if _, authorized := s.eventStreamAuthorized(request, topologyID); !authorized { return } if _, err := fmt.Fprint(w, ": ping\n\n"); err != nil { @@ -1671,6 +1765,19 @@ func (s *Server) events(w http.ResponseWriter, request *http.Request) { } } +// commentAuthor attributes a comment to the signed-in account. The free-text +// author is used only when the server runs without authentication. +func (s *Server) commentAuthor(request *http.Request, provided string) string { + session, authenticated := s.sessionFromRequest(request) + if !authenticated { + return strings.TrimSpace(provided) + } + if session.Principal.IsGuest() { + return "Guest" + } + return session.Principal.Username +} + // acquireSessionStream reserves one of the session's event-stream slots. Without // authentication there is no session to account against. func (s *Server) acquireSessionStream(request *http.Request) (func(), bool) { @@ -1702,16 +1809,16 @@ func (s *Server) acquireSessionStream(request *http.Request) (func(), bool) { // eventStreamAuthorized re-checks an open stream on every event and heartbeat. // It reads only the owning organization so idle streams never reload, decode // and validate the full document. -func (s *Server) eventStreamAuthorized(request *http.Request, topologyID string) bool { +func (s *Server) eventStreamAuthorized(request *http.Request, topologyID string) (string, bool) { organizationID, err := s.store.TopologyOrganizationID(request.Context(), topologyID) if err != nil { - return false + return "", false } if s.auth == nil { - return true + return organizationID, true } session, authenticated := s.sessionFromRequest(request) - return authenticated && s.auth.CanAccessTopology(session.Principal, topologyID, organizationID) + return organizationID, authenticated && s.auth.CanAccessTopology(session.Principal, topologyID, organizationID) } func (s *Server) staticFile(w http.ResponseWriter, request *http.Request) { @@ -1762,7 +1869,7 @@ func (s *Server) serveStaticContent(w http.ResponseWriter, request *http.Request } func (s *Server) publish(id, eventType string, topology model.Topology) { - if err := s.broker.Publish(id, eventType, topology); err != nil { + if err := s.broker.PublishScoped(id, topology.OrganizationID, eventType, publicTopology(topology)); err != nil { s.logger.Error("publishing sse event", "event", eventType, "error", err) } } @@ -1776,8 +1883,8 @@ func (s *Server) mutate(request *http.Request, id string, mutation func(*model.T // Detached photo files are removed after the commit and outside the // process-wide lock, so slow storage never delays other organizations. updated, err := func() (model.Topology, error) { - s.directoryMu.Lock() - defer s.directoryMu.Unlock() + s.directoryMu.RLock() + defer s.directoryMu.RUnlock() return s.store.MutateAtRevision(request.Context(), id, expectedRevision, func(topology *model.Topology) error { if !s.authorizedTopologySnapshot(request, *topology) { return store.ErrNotFound diff --git a/internal/handler/server_test.go b/internal/handler/server_test.go index 4216f08..345a864 100644 --- a/internal/handler/server_test.go +++ b/internal/handler/server_test.go @@ -1205,6 +1205,10 @@ func requestTopology( } request := httptest.NewRequestWithContext(t.Context(), method, path, bytes.NewReader(data)) request.Header.Set("Content-Type", "application/json") + if topology, replacing := body.(model.Topology); replacing && method == http.MethodPut { + // Whole-document replaces must name their base revision, as the SPA does. + request.Header.Set("If-Match", topologyRevisionETag(topology.Revision)) + } response := httptest.NewRecorder() handler.ServeHTTP(response, request) if response.Code != expectedStatus { diff --git a/internal/media/store.go b/internal/media/store.go index f4a669b..1e22797 100644 --- a/internal/media/store.go +++ b/internal/media/store.go @@ -9,9 +9,11 @@ import ( "image/jpeg" "image/png" "io" + "io/fs" "net/http" "os" "path" + "regexp" ) const ( @@ -179,3 +181,67 @@ func extensionForMediaType(mediaType string) (string, bool) { return "", false } } + +// Reference names one stored photo that Sweep must keep. +type Reference struct { + TopologyID string + PhotoID string + MediaType string +} + +var topologyDirectoryPattern = regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$`) + +// Sweep removes photo files and topology directories that no reference names. +// Uploads write files before the authorizing commit, so a crash in between, or +// a failed removal, leaves files no document points to. Only topology-shaped +// directories are considered; anything else in the media root is left alone. +// Call it only with the complete reference set and while no uploads run. +func (s *Store) Sweep(references []Reference) (int, error) { + keep := make(map[string]map[string]struct{}) + for _, reference := range references { + extension, ok := extensionForMediaType(reference.MediaType) + if !ok { + continue + } + if keep[reference.TopologyID] == nil { + keep[reference.TopologyID] = make(map[string]struct{}) + } + keep[reference.TopologyID][reference.PhotoID+extension] = struct{}{} + } + entries, err := fs.ReadDir(s.root.FS(), ".") + if err != nil { + return 0, fmt.Errorf("listing media directory: %w", err) + } + removed := 0 + for _, entry := range entries { + name := entry.Name() + if !entry.IsDir() || !topologyDirectoryPattern.MatchString(name) { + continue + } + files, referenced := keep[name] + if !referenced { + if err := s.root.RemoveAll(name); err != nil { + return removed, fmt.Errorf("removing orphaned topology media: %w", err) + } + removed++ + continue + } + children, err := fs.ReadDir(s.root.FS(), name) + if err != nil { + return removed, fmt.Errorf("listing topology media: %w", err) + } + for _, child := range children { + if child.IsDir() { + continue + } + if _, ok := files[child.Name()]; ok { + continue + } + if err := s.root.Remove(name + "/" + child.Name()); err != nil && !errors.Is(err, os.ErrNotExist) { + return removed, fmt.Errorf("removing orphaned photo: %w", err) + } + removed++ + } + } + return removed, nil +} diff --git a/internal/media/store_test.go b/internal/media/store_test.go index c7defc3..8a537c7 100644 --- a/internal/media/store_test.go +++ b/internal/media/store_test.go @@ -9,6 +9,7 @@ import ( "image/png" "io" "os" + "path/filepath" "testing" ) @@ -102,3 +103,48 @@ func validJPEG(t *testing.T) []byte { } return output.Bytes() } + +func TestSweepRemovesOnlyUnreferencedTopologyMedia(t *testing.T) { + t.Parallel() + directory := t.TempDir() + store, err := Open(directory) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = store.Close() }) + const kept = "11111111-1111-4111-8111-111111111111" + const orphanTopology = "22222222-2222-4222-8222-222222222222" + files := map[string]string{ + kept + "/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa.png": "keep", + kept + "/bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb.png": "orphan photo", + orphanTopology + "/cccccccc-cccc-4ccc-8ccc-cccccccccccc.jpg": "orphan topology", + "lost+found/unrelated": "not media", + } + for name, content := range files { + path := filepath.Join(directory, filepath.FromSlash(name)) + if err := os.MkdirAll(filepath.Dir(path), 0o750); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + } + removed, err := store.Sweep([]Reference{{TopologyID: kept, PhotoID: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", MediaType: "image/png"}}) + if err != nil { + t.Fatal(err) + } + if removed != 2 { + t.Fatalf("removed = %d, want 2", removed) + } + for name, wantExists := range map[string]bool{ + kept + "/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa.png": true, + kept + "/bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb.png": false, + orphanTopology: false, + "lost+found/unrelated": true, + } { + _, statErr := os.Stat(filepath.Join(directory, filepath.FromSlash(name))) + if exists := statErr == nil; exists != wantExists { + t.Errorf("%s exists = %t, want %t", name, exists, wantExists) + } + } +} diff --git a/internal/model/analyzer.go b/internal/model/analyzer.go index b5c99df..214e8c9 100644 --- a/internal/model/analyzer.go +++ b/internal/model/analyzer.go @@ -331,20 +331,32 @@ func TracePath(topology Topology, sourcePortID, targetPortID string, vlanID int) deviceID: left.DeviceID, linkID: link.ID, fromPortID: right.ID, toPortID: left.ID, }) } - type visit struct { - deviceID string - path []string + // Each device records the link it was reached through, and the path is + // rebuilt once at the target, so the search uses linear memory instead of + // copying a growing path for every queued device. + type arrival struct { + previous string + linkID string } - queue := []visit{{deviceID: source.DeviceID, path: []string{}}} + arrivals := map[string]arrival{} + rebuild := func(last string, linkID string) []string { + path := []string{linkID} + for current := last; current != source.DeviceID; current = arrivals[current].previous { + path = append(path, arrivals[current].linkID) + } + slices.Reverse(path) + return path + } + queue := []string{source.DeviceID} seen := map[string]struct{}{source.DeviceID: {}} for len(queue) > 0 { - current := queue[0] + currentID := queue[0] queue = queue[1:] - if current.deviceID != source.DeviceID && categories[current.deviceID] == DeviceCategoryServer { + if currentID != source.DeviceID && categories[currentID] == DeviceCategoryServer { continue } - for _, next := range adjacency[current.deviceID] { - leavesSourceServer := current.deviceID == source.DeviceID && sourceIsServer + for _, next := range adjacency[currentID] { + leavesSourceServer := currentID == source.DeviceID && sourceIsServer if leavesSourceServer && next.fromPortID != sourcePortID { continue } @@ -355,12 +367,12 @@ func TracePath(topology Topology, sourcePortID, targetPortID string, vlanID int) if _, visited := seen[next.deviceID]; visited { continue } - path := append(slices.Clone(current.path), next.linkID) if next.deviceID == target.DeviceID { - return path, nil + return rebuild(currentID, next.linkID), nil } seen[next.deviceID] = struct{}{} - queue = append(queue, visit{deviceID: next.deviceID, path: path}) + arrivals[next.deviceID] = arrival{previous: currentID, linkID: next.linkID} + queue = append(queue, next.deviceID) } } return nil, fmt.Errorf("no path carries vlan %d", vlanID) @@ -368,8 +380,16 @@ func TracePath(topology Topology, sourcePortID, targetPortID string, vlanID int) func missingVLANs(tagged []int, peer Port) []int { missing := []int{} + if peer.Mode == PortModeUnconfigured { + return append(missing, tagged...) + } + carried := make(map[int]struct{}, len(peer.AllowedVLANs)) + carried[peer.NativeVLAN] = struct{}{} + for _, vlanID := range peer.AllowedVLANs { + carried[vlanID] = struct{}{} + } for _, vlanID := range tagged { - if !portCarriesVLAN(peer, vlanID) { + if _, ok := carried[vlanID]; !ok { missing = append(missing, vlanID) } } diff --git a/internal/model/stp.go b/internal/model/stp.go index d1270cb..a530431 100644 --- a/internal/model/stp.go +++ b/internal/model/stp.go @@ -93,8 +93,9 @@ func SimulateSTP(topology Topology, ports map[string]Port) []STPInstance { bridgeByDevice, bridges := stpLogicalBridges(topology) instances := []STPInstance{} pathBudget := maxSTPPathEntries + linkIndex := newSTPLinkIndex(topology) for _, vlan := range topology.VLANs { - edges := stpEdges(topology, ports, bridgeByDevice, vlan.ID) + edges := stpEdges(topology, ports, bridgeByDevice, vlan.ID, linkIndex) components := stpComponents(edges) connectedBridges := make(map[string]struct{}) for domain, component := range components { @@ -199,17 +200,30 @@ func normalizedSTPPriority(priority int) int { return priority } -func stpEdges(topology Topology, ports map[string]Port, bridgeByDevice map[string]string, vlanID int) []stpEdge { - links := make(map[string]Link, len(topology.Links)) - groupByLink := make(map[string]LinkGroup) +// stpLinkIndex is built once per analysis; stpEdges runs once per VLAN. +type stpLinkIndex struct { + links map[string]Link + groupByLink map[string]LinkGroup +} + +func newSTPLinkIndex(topology Topology) stpLinkIndex { + index := stpLinkIndex{ + links: make(map[string]Link, len(topology.Links)), + groupByLink: make(map[string]LinkGroup), + } for _, link := range topology.Links { - links[link.ID] = link + index.links[link.ID] = link } for _, group := range topology.LinkGroups { for _, linkID := range group.LinkIDs { - groupByLink[linkID] = group + index.groupByLink[linkID] = group } } + return index +} + +func stpEdges(topology Topology, ports map[string]Port, bridgeByDevice map[string]string, vlanID int, index stpLinkIndex) []stpEdge { + links, groupByLink := index.links, index.groupByLink eligible := func(link Link) (string, string, bool) { left, leftExists := bridgeByDevice[link.SourceDeviceID] right, rightExists := bridgeByDevice[link.TargetDeviceID] diff --git a/internal/model/validation.go b/internal/model/validation.go index 7c8f30b..8b3f903 100644 --- a/internal/model/validation.go +++ b/internal/model/validation.go @@ -32,6 +32,8 @@ const ( MaxTopologyLinkGroups = 5000 MaxTopologySwitchSystems = 2000 MaxTopologyFirewallGroups = 2000 + // MaxTopologyPhotoBytes caps the combined size of one topology's photos. + MaxTopologyPhotoBytes = 1 << 30 // MaxTopologyDocumentBytes caps the encoded document a store persists. MaxTopologyDocumentBytes = 16 << 20 ) @@ -73,6 +75,13 @@ func (t Topology) validateCardinality() error { if totalPorts > MaxTopologyPorts { return fmt.Errorf("topology may contain at most %d ports", MaxTopologyPorts) } + var photoBytes int64 + for _, photo := range t.Photos { + photoBytes += max(photo.SizeBytes, 0) + } + if photoBytes > MaxTopologyPhotoBytes { + return fmt.Errorf("topology photos may total at most %d bytes", int64(MaxTopologyPhotoBytes)) + } return nil } diff --git a/internal/sse/broker.go b/internal/sse/broker.go index a22d466..7f5f9ef 100644 --- a/internal/sse/broker.go +++ b/internal/sse/broker.go @@ -23,6 +23,9 @@ var ErrSubscriberLimit = errors.New("sse: subscriber limit reached") type Event struct { Type string Data []byte + // OrganizationID is the organization that owned the topology when the + // event was produced; empty when the publisher did not scope it. + OrganizationID string } // Broker owns subscribers grouped by topology ID. @@ -89,11 +92,16 @@ func (b *Broker) TrySubscribe(topologyID string) (<-chan Event, func(), error) { // Publish sends an event without allowing a slow subscriber to block a mutation. func (b *Broker) Publish(topologyID, eventType string, payload any) error { + return b.PublishScoped(topologyID, "", eventType, payload) +} + +// PublishScoped is Publish for an event tagged with its owning organization. +func (b *Broker) PublishScoped(topologyID, organizationID, eventType string, payload any) error { data, err := json.Marshal(payload) if err != nil { return err } - event := Event{Type: eventType, Data: data} + event := Event{Type: eventType, Data: data, OrganizationID: organizationID} b.mu.Lock() defer b.mu.Unlock() for channel := range b.subscribers[topologyID] { diff --git a/internal/store/database.go b/internal/store/database.go index 513dbd2..e45d483 100644 --- a/internal/store/database.go +++ b/internal/store/database.go @@ -2,6 +2,7 @@ package store import ( "context" + "errors" "fmt" "time" @@ -35,3 +36,33 @@ func OpenDatabase(ctx context.Context, url string) (*pgxpool.Pool, error) { } return pool, nil } + +// instanceLockKey is the session-level advisory lock that marks the single +// WireDraft process allowed to use a database. +const instanceLockKey int64 = 0x5749524544524654 // "WIREDRFT" + +// ErrInstanceRunning reports that another WireDraft process holds the lock. +var ErrInstanceRunning = errors.New("another WireDraft instance is already using this database") + +// AcquireInstanceLock takes a dedicated connection out of the pool and holds a +// session advisory lock on it for the life of the process. Sessions and the +// authentication state are process-local and written whole, so a second +// replica would silently lose updates; this makes it fail at startup instead. +// The lock is released by the returned function or when the process exits. +func AcquireInstanceLock(ctx context.Context, pool *pgxpool.Pool) (func(), error) { + pooled, err := pool.Acquire(ctx) + if err != nil { + return nil, fmt.Errorf("acquiring instance lock connection: %w", err) + } + conn := pooled.Hijack() + var acquired bool + if err := conn.QueryRow(ctx, `SELECT pg_try_advisory_lock($1)`, instanceLockKey).Scan(&acquired); err != nil { + _ = conn.Close(context.Background()) + return nil, fmt.Errorf("taking instance lock: %w", err) + } + if !acquired { + _ = conn.Close(context.Background()) + return nil, ErrInstanceRunning + } + return func() { _ = conn.Close(context.Background()) }, nil +} diff --git a/internal/store/postgres.go b/internal/store/postgres.go index 1ed25a8..467d3a8 100644 --- a/internal/store/postgres.go +++ b/internal/store/postgres.go @@ -502,7 +502,7 @@ func (s *PostgresStore) MutateAtRevision( rack_count = $7, device_count = $8, link_count = $9, updated_at = $10, document = $11::jsonb WHERE id = $1 AND revision = $12`, - next.ID, next.Name, next.OrganizationID, next.Organization, next.Location, next.Revision, + id, next.Name, next.OrganizationID, next.Organization, next.Location, next.Revision, len(next.Racks), next.LogicalDeviceCount(), len(next.Links), next.UpdatedAt, nextDocument, actualRevision, ) diff --git a/web/static/index.html b/web/static/index.html index e14d8b0..9b15b53 100644 --- a/web/static/index.html +++ b/web/static/index.html @@ -251,7 +251,7 @@

NO ENDPOINT SELECTED

DOCUMENTATION

EMBED + OPEN
- +
diff --git a/web/static/js/app.js b/web/static/js/app.js index 13f1686..86df4d1 100644 --- a/web/static/js/app.js +++ b/web/static/js/app.js @@ -821,7 +821,7 @@ function renderPhotoInspector(selection) { section.innerHTML = `

FIELD PHOTOS

${photos.length} PHOTO${photos.length === 1 ? "" : "S"}
- ${photos.map((photo) => ``).join("")} + ${photos.map((photo) => ``).join("")} ${photos.length ? "" : `

No protected photos are attached to this ${escapeHTML(selection.type)}.

`}
@@ -871,7 +871,7 @@ function renderPhotoManager() { photo.targetKind === active.targetKind && photo.targetId === active.targetId); elements["photo-manager-count"].textContent = `${photos.length} PHOTO${photos.length === 1 ? "" : "S"}`; elements["photo-manager-list"].innerHTML = photos.map((photo) => ` - `).join(""); elements["photo-preview"].src = photoURL(active); @@ -967,11 +967,11 @@ function renderCommentInspector(selection) { ${threads.length ? threads.map((thread) => `
${thread.resolved ? "RESOLVED" : "ACTIVE"}
${(thread.messages || []).map((message) => `

${escapeHTML(message.author)}${escapeHTML(message.body)}

`).join("")}
-
+
`).join("") : `

No comments on this ${escapeHTML(anchor.kind)} yet.

`} - +
`; @@ -986,6 +986,12 @@ function planCommentTimestamp(value) { return new Intl.DateTimeFormat(undefined, { dateStyle: "medium", timeStyle: "short" }).format(date); } +/** Comments are attributed to the signed-in account by the server; show that name. */ +function commentAuthorName() { + if (sessionInfo?.role === "guest") return "Guest"; + return sessionInfo?.username || "Operator"; +} + async function savePlanComment(event, anchor) { event.preventDefault(); const form = new FormData(event.currentTarget); @@ -1165,7 +1171,7 @@ function renderRackInspector(rackID) { ${roomSelectMarkup(rack.roomId)} - +
`; document.getElementById("rack-inspector-form").addEventListener("submit", async (event) => { @@ -2111,11 +2117,11 @@ function renderResources() { elements["documentation-list"].innerHTML = documents.length ? documents.map((item) => `
${escapeHTML(item.targetKind.toUpperCase())}${escapeHTML(item.label)}

${escapeHTML(item.url)}

-
${embeddableDocumentationURL(item.url) ? `` : ""}OPEN ↗
+
${embeddableDocumentationURL(item.url) ? `` : ""}OPEN ↗
`).join("") : `

No documentation attached.

`; const created = createdShareURL ? `` : ""; - elements["share-list"].innerHTML = created + (shareEntries.length ? shareEntries.map((share) => `
${escapeHTML(share.name)}${share.expiresAt ? `EXPIRES ${escapeHTML(new Date(share.expiresAt).toLocaleString())}` : "NO EXPIRY"}
`).join("") : `

No active read-only shares.

`); + elements["share-list"].innerHTML = created + (shareEntries.length ? shareEntries.map((share) => `
${escapeHTML(share.name)}${share.expiresAt ? `EXPIRES ${escapeHTML(new Date(share.expiresAt).toLocaleString())}` : "NO EXPIRY"}
`).join("") : `

No active read-only shares.

`); } // Only HTTPS documentation is framed (the CSP allows frame-src https: only); diff --git a/web/static/js/export.js b/web/static/js/export.js index 60f3747..95e8c6c 100644 --- a/web/static/js/export.js +++ b/web/static/js/export.js @@ -526,8 +526,8 @@ export function buildSVGDocument(topology, engine) { for (const box of rackBoxes) { const x = box.x + offsetX; const y = box.y + offsetY; parts.push(`${escapeXML(box.rack.name)} · ${box.rack.heightU}U rack`); - parts.push(``); - parts.push(``); + parts.push(``); + parts.push(``); parts.push(`${escapeXML(box.rack.name)}`); parts.push(`${box.rack.heightU}U RACK`); for (let unit = 0; unit <= box.rack.heightU; unit += 1) { @@ -578,7 +578,7 @@ export function buildSVGDocument(topology, engine) { const statusX = x + statusArea.x * 690; const statusY = y + statusArea.y * heightU - statusArea.height / 2; const rackFace = device.rackId ? (box.face || device.rackFace || "front") : "free"; - parts.push(`${escapeXML(device.name)} · ${escapeXML(device.model || device.category || "Device")}`); + parts.push(`${escapeXML(device.name)} · ${escapeXML(device.model || device.category || "Device")}`); if (scene?.profile) { const chassis = scene.chassis; // A strict model opt-in replaces the generic body with its authored component silhouette. @@ -589,7 +589,7 @@ export function buildSVGDocument(topology, engine) { } if (scene.portal) { const portal = scene.portal; - renderedPanelPortals.push(`${escapeXML(portal.label)}${escapeXML(portal.label)}`); + renderedPanelPortals.push(`${escapeXML(portal.label)}${escapeXML(portal.label)}`); } parts.push(``); } else { @@ -730,7 +730,7 @@ export function buildSVGDocument(topology, engine) { const y = box.y + box.height / 2 + offsetY; const badgeLink = renderedLinkByID.get(badge.linkId); parts.push(``); - parts.push(``); + parts.push(``); parts.push(`${badge.role}`); } const endpointBadges = layoutEndpointBadges(renderedLinks.flatMap((entry) => entry.badges), {