From 4b30d4a44efc1203f2679c05f13f54e2eae0dfe2 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 19:50:08 +0000 Subject: [PATCH] =?UTF-8?q?feat(connections):=20edit=20connections=20via?= =?UTF-8?q?=20PUT=20=E2=80=94=20backend=20+=20UI=20+=20blank-secret=20merg?= =?UTF-8?q?e?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backend: PUT /api/connections/{id} with UpdateConnectionRequest {Name, Config}. Provider is immutable; a secret field left blank in the incoming config is merged from the stored row so users can rename or rotate non-secret fields without re-typing every credential. Invalidates the ConnectionInstanceRegistry cache on update so the next GetConnection() re-opens with the fresh config. 409 on unique-name-per-provider collision, 404 on unknown id, 400 on non- object config. 6 new xUnit tests cover the merge, overwrite, 404, 409, 400 paths plus the happy-path round-trip. Frontend: ConnectionsPage gains an Edit pencil per row. The form is shared between create and edit modes; provider select is disabled in edit; secret fields render blank with an inline hint that leaving them blank keeps the stored value. One new Vitest covers the edit flow. --- docs/superpowers/plan-4-kickoff-prompt.md | 108 ++++++++++++++ frontend/src/pages/ConnectionsPage.test.tsx | 38 +++++ frontend/src/pages/ConnectionsPage.tsx | 96 +++++++++--- .../Contracts/ConnectionDtos.cs | 1 + .../Endpoints/ConnectionsEndpoints.cs | 50 +++++++ .../Repositories/ConnectionRepository.cs | 16 ++ .../ConnectionsEndpointsTests.cs | 139 ++++++++++++++++++ 7 files changed, 428 insertions(+), 20 deletions(-) create mode 100644 docs/superpowers/plan-4-kickoff-prompt.md diff --git a/docs/superpowers/plan-4-kickoff-prompt.md b/docs/superpowers/plan-4-kickoff-prompt.md new file mode 100644 index 0000000..553ce85 --- /dev/null +++ b/docs/superpowers/plan-4-kickoff-prompt.md @@ -0,0 +1,108 @@ +Continue VolcanicCore development. Plans 1, 2, and 3 are shipped and pushed to +origin. Head is `7854cb0` on `main`; the last plan-close commit is +`docs: close out Plan 3 — React SPA live`. Backend is 74/74 (`dotnet test`), +frontend is 20/20 (`pnpm --dir frontend test`), and the Playwright happy-path +smoke passes (`pnpm --dir frontend test:e2e`). + +Read the following in order before writing anything: +- `CLAUDE.md` — current status, invariants (esp. the SPA-bundles-into-wwwroot + and MapFallbackToFile-goes-last rules), and the working style that shipped + Plans 1, 2, and 3 cleanly (superpowers:brainstorming for design if the shape + isn't obvious, superpowers:writing-plans for the plan doc, + superpowers:subagent-driven-development for execution — auto mode, no + per-task confirmation). +- `docs/superpowers/specs/2026-08-28-volcaniccore-workflow-engine-design.md` — + §8 (Deployment) and §1 (Architecture) especially. The design already commits + to single-container Coolify, `/data` volume, single ASP.NET Core 9 process. +- `docs/superpowers/plans/2026-08-29-plan-2-workflow-engine.md` and + `docs/superpowers/plans/2026-08-30-plan-3-react-spa.md` — for cadence and + file-structure conventions. +- `docs/superpowers/followups.md` — cross-plan items. Two Plan-4 items are + already listed (forwarded-headers/secure-cookie behind Traefik + boot-time + re-seed on legacy plaintext `admin.password_hash_source`). Plan 4 MUST clear + both. + +Read these skills before authoring: +- `preparing-dotnet-react-app-for-coolify` — Dockerfile pattern, docker-compose + conventions, `expose: [8080]`, `SERVICE_FQDN_APP`, named volume, static-file + serving env-var overrides. +- `deploying-to-coolify-via-api` — v4 REST API for creating/updating apps, + `deploy.json` for non-secret config, project/server discovery, HTTP-probe + verification. + +Author **Plan 4 — Container + Coolify deploy** using superpowers:writing-plans, +then execute it with superpowers:subagent-driven-development (same cadence as +Plans 1, 2, 3 — auto mode, no per-task confirmation, TDD per task where it +applies, two-stage reviews for substantive tasks and combined lightweight +reviews for mechanical ones, conventional-commits messages). + +Plan 4 must cover, at minimum: + +- **Multi-stage Dockerfile** in the repo root: pnpm-installs + Vite build the + SPA into a scratch stage's `/build/wwwroot`, `dotnet publish` the API in a + separate stage using `sdk:9.0`, final runtime image based on the .NET 9 + runtime (or aspnet:9.0 — distroless if it plays nicely with IronPython's + P/Invoke; if not, `mcr.microsoft.com/dotnet/aspnet:9.0` is fine — call it out + in the plan). Copy the SPA build output into `/app/wwwroot`. `EXPOSE 8080`. + `ENTRYPOINT ["dotnet", "VolcanicCore.Api.dll"]`. +- **docker-compose.yml** at repo root, Coolify-flavored per the skill: single + service, `expose: [8080]`, named volume mounted at `/data`, env passed + through (`ADMIN_PASSWORD`, `ENCRYPTION_KEY`, retention/timeout knobs, + `DATA_DIRECTORY=/data`), `SERVICE_FQDN_APP` for the domain, Traefik labels + only when the domain is set. +- **`Program.cs` — `UseForwardedHeaders` wired before `UseAuthentication`.** + Configure `ForwardedHeaders.XForwardedProto | XForwardedFor`, + `KnownNetworks` + `KnownProxies` cleared so the Coolify/Traefik reverse + proxy is trusted inside the container network. Cookie config stays + `SameSite=Lax` but `Secure` starts flipping to `Always` behind TLS. Add + xUnit tests that prove the middleware sees `X-Forwarded-Proto: https` and + reports `Request.Scheme == "https"`. +- **Boot-time re-seed on legacy plaintext `admin.password_hash_source`.** + Wrap the `repo.Get(PasswordSourceKey)` call in `AdminBootstrap.Seed` in a + `try/catch (CryptographicException)` — treat "can't decrypt" as "row is in + the pre-Plan-2 plaintext format, treat as unset" so the boot re-seeds + cleanly. Add a test that seeds a plaintext row directly into `admin_config` + and asserts `AdminBootstrap.Seed` fixes it up on next boot. +- **`deploy.json`** at repo root (or under a `deploy/` dir) capturing project + UUID, server UUID, planned domain, non-secret app env vars — populated once + from Coolify project/server discovery (Task in the plan asks the user for + the domain). +- **Coolify deploy — CREATE the app via v4 API** if it doesn't exist yet, PATCH + its git config + env vars, then trigger a deploy fire-and-forget per the + global CLAUDE.md rule. Verify the deployment id came back; DO NOT poll the + build. Instead, the plan's Playwright smoke can be reused later against + the live URL as a follow-up sanity — do NOT block on it in this plan. +- **CI story for the Playwright smoke.** If GitHub Actions / Gitea Actions + aren't wired yet, add a lightweight workflow (or note in followups.md that + it lands in a Plan-5 CI task). Either way, make sure the Playwright browser + install step is documented. +- **README-ish** — the plan's close-out task should extend `CLAUDE.md` with the + live URL, the current deploy invocation, and any new invariants the Docker + build introduces. + +After Plan 4 executes cleanly, the app is live at its Coolify FQDN and the +close-out commit is on `main`. This is the end of the initial-launch scope. + +Working invariants that MUST NOT be relearned from scratch: +- Working branch: `main`. No feature branches. +- Every commit needs `-c user.email="claude@volcanic" -c user.name="Claude"`. +- All the Plan-1/2/3 invariants in CLAUDE.md: cookie auth is in the .NET 9 + shared framework (do NOT add `Microsoft.AspNetCore.Authentication.Cookies` + as a PackageReference); test parallelism stays off; env-var loop honors + pre-set config; `WebhookRegistry` / `WebhookRouter` live in `.Core`; + `TriggerActivationSink` is fire-and-forget; SPA bundles into + `src/VolcanicCore.Api/wwwroot/`; `MapFallbackToFile` stays the LAST `Map*` + call; `vite.config.ts` needs `/// ` on line 1. +- Global CLAUDE.md: deploys are fire-and-forget via the Coolify v4 API — do + NOT poll the build, do NOT block on the FQDN. The API returns a deployment + id; that's the success signal. + +Env vars available in the session: `COOLIFY_URL`, `COOLIFY_KEY`, `GIT_URL`, +`GIT_USER`, `GIT_KEY`. Use the `deploying-to-coolify-via-api` skill for the +splice-credentials-into-git-URL and stderr-redaction patterns. + +Start by pulling latest, reading CLAUDE.md + followups + the two skills above, +confirming `dotnet test` is 74/74 and `pnpm --dir frontend test` is 20/20 +before authoring Plan 4. Ask the user for the intended Coolify domain +(e.g. `volcaniccore.tes.gd`) BEFORE the plan gets to the `deploy.json` +creation task — that's a decision only they can make. diff --git a/frontend/src/pages/ConnectionsPage.test.tsx b/frontend/src/pages/ConnectionsPage.test.tsx index 05cc031..e5fc655 100644 --- a/frontend/src/pages/ConnectionsPage.test.tsx +++ b/frontend/src/pages/ConnectionsPage.test.tsx @@ -29,6 +29,44 @@ describe("ConnectionsPage", () => { expect(screen.getByText(/nats/i)).toBeInTheDocument(); }); + it("edits an existing connection via PUT, preserving blank secret", async () => { + let latestPut: { name?: string; config?: { url?: string; pass?: string } } | null = null; + mswServer.use( + http.get("/api/connection-providers", () => HttpResponse.json(providers)), + http.get("/api/connections", () => + HttpResponse.json([ + { id: "c1", providerId: "nats", name: "local", config: { url: "nats://old" } }, + ] as never)), + http.put("/api/connections/c1", async ({ request }) => { + latestPut = await request.json() as never; + return HttpResponse.json({ + id: "c1", providerId: "nats", name: latestPut!.name, config: { url: latestPut!.config?.url }, + } as never); + }), + ); + renderWithProviders(); + await userEvent.click(await screen.findByRole("button", { name: /^edit$/i })); + // Provider select must be disabled in edit mode (provider is immutable). + expect(screen.getByLabelText(/provider/i)).toBeDisabled(); + // Rename + change URL; leave password blank (secret stays via backend merge). + const nameInput = screen.getByLabelText(/^name$/i) as HTMLInputElement; + await userEvent.clear(nameInput); + await userEvent.type(nameInput, "local-renamed"); + const urlInput = screen.getByLabelText(/server url/i) as HTMLInputElement; + await userEvent.clear(urlInput); + await userEvent.type(urlInput, "nats://new"); + await userEvent.click(screen.getByRole("button", { name: /^save$/i })); + await waitFor(() => { + expect(latestPut).not.toBeNull(); + expect(latestPut!.name).toBe("local-renamed"); + expect(latestPut!.config?.url).toBe("nats://new"); + // Password field left blank in the form — the frontend sends nothing (or "") + // and the backend merges the stored secret. Either way, no plaintext leaks + // from the frontend to the wire in the "leave blank" flow. + expect(latestPut!.config?.pass ?? "").toBe(""); + }); + }); + it("creates a new connection using the provider's schema", async () => { let created: unknown = null; mswServer.use( diff --git a/frontend/src/pages/ConnectionsPage.tsx b/frontend/src/pages/ConnectionsPage.tsx index 65aaa49..06c544e 100644 --- a/frontend/src/pages/ConnectionsPage.tsx +++ b/frontend/src/pages/ConnectionsPage.tsx @@ -1,50 +1,91 @@ import { useState } from "react"; -import { Plus, Trash2 } from "lucide-react"; +import { Pencil, Plus, Trash2 } from "lucide-react"; import { apiFetch } from "@/api/client"; import { qk, useApiMutation, useConnections, useInvalidate, useProviders } from "@/api/hooks"; import { SchemaForm } from "@/forms/SchemaForm"; -import type { ConnectionDto } from "@/api/types"; +import type { ConnectionDto, JsonSchemaObject } from "@/api/types"; + +type ProviderDto = { id: string; displayName: string; configSchema: JsonSchemaObject }; export default function ConnectionsPage() { const providers = useProviders(); const conns = useConnections(); const invalidate = useInvalidate(); - const [creating, setCreating] = useState(false); + + // null = closed; "new" = create form; string = editing that connection id. + const [openForm, setOpenForm] = useState(null); const create = useApiMutation( (body: { providerId: string; name: string; config: Record }) => apiFetch("/api/connections", { method: "POST", body }), - { onSuccess: () => { invalidate(qk.connections); setCreating(false); } }, + { onSuccess: () => { invalidate(qk.connections); setOpenForm(null); } }, + ); + const update = useApiMutation( + (input: { id: string; body: { name: string; config: Record } }) => + apiFetch(`/api/connections/${input.id}`, { method: "PUT", body: input.body }), + { onSuccess: () => { invalidate(qk.connections); setOpenForm(null); } }, ); const remove = useApiMutation( (id: string) => apiFetch(`/api/connections/${id}`, { method: "DELETE" }), { onSuccess: () => invalidate(qk.connections) }, ); + const editing = openForm && openForm !== "new" + ? conns.data?.find((c) => c.id === openForm) ?? null + : null; + return (

Connections

- {creating && providers.data && ( - create.mutate(body)} - onCancel={() => setCreating(false)} + initialProviderId={providers.data[0]?.id ?? ""} + initialName="" + initialConfig={{}} submitting={create.isPending} + onSubmit={(body) => create.mutate(body)} + onCancel={() => setOpenForm(null)} /> )} + + {editing && providers.data && ( + } + submitting={update.isPending} + onSubmit={(body) => + update.mutate({ id: editing.id, body: { name: body.name, config: body.config } }) + } + onCancel={() => setOpenForm(null)} + /> + )} +
    {conns.data?.map((c) => (
  • {c.name} {c.providerId} +
diff --git a/src/VolcanicCore.Api/Contracts/ConnectionDtos.cs b/src/VolcanicCore.Api/Contracts/ConnectionDtos.cs index 1b2b291..62c03ce 100644 --- a/src/VolcanicCore.Api/Contracts/ConnectionDtos.cs +++ b/src/VolcanicCore.Api/Contracts/ConnectionDtos.cs @@ -3,4 +3,5 @@ using System.Text.Json; namespace VolcanicCore.Api.Contracts; public sealed record CreateConnectionRequest(string ProviderId, string Name, JsonElement Config); +public sealed record UpdateConnectionRequest(string Name, JsonElement Config); public sealed record ConnectionResponse(string Id, string ProviderId, string Name, JsonElement Config); diff --git a/src/VolcanicCore.Api/Endpoints/ConnectionsEndpoints.cs b/src/VolcanicCore.Api/Endpoints/ConnectionsEndpoints.cs index 8ef62ba..040b874 100644 --- a/src/VolcanicCore.Api/Endpoints/ConnectionsEndpoints.cs +++ b/src/VolcanicCore.Api/Endpoints/ConnectionsEndpoints.cs @@ -47,6 +47,36 @@ public static class ConnectionsEndpoints } }); + g.MapPut("/{id}", async (string id, UpdateConnectionRequest req, + ConnectionRepository repo, IConnectionSecretPolicy policy, + ConnectionInstanceRegistry instances) => + { + if (req.Config.ValueKind != JsonValueKind.Object) + return Results.BadRequest(new { error = "config must be a JSON object" }); + + var existing = repo.Get(id); + if (existing is null) return Results.NotFound(); + + var secrets = policy.SecretFieldsFor(existing.ProviderId); + var merged = MergeSecrets(existing.ConfigJson, req.Config, secrets); + + try + { + if (!repo.Update(id, req.Name, merged)) return Results.NotFound(); + } + catch (Microsoft.Data.Sqlite.SqliteException ex) when (ex.SqliteErrorCode == 19) + { + return Results.Conflict(new { error = "a connection with that name already exists for this provider" }); + } + + // Config changed — drop the pooled live instance so the next + // GetConnection() re-opens with the updated config. + await instances.InvalidateAsync(id); + + var stripped = StripSecrets(merged, secrets); + return Results.Ok(new ConnectionResponse(id, existing.ProviderId, req.Name, stripped)); + }); + g.MapDelete("/{id}", async (string id, ConnectionRepository repo, ConnectionInstanceRegistry instances) => { await instances.InvalidateAsync(id); @@ -54,6 +84,26 @@ public static class ConnectionsEndpoints }); } + // For each field in `secrets`, if the incoming config lacks that key OR + // the value is an empty string, use the stored value from `existingJson`. + // Non-secret fields always come from the incoming config. This gives + // "leave blank to keep the existing secret" without exposing it on GET. + private static string MergeSecrets(string existingJson, JsonElement incoming, IReadOnlySet secrets) + { + using var existingDoc = JsonDocument.Parse(existingJson); + var merged = new Dictionary(); + foreach (var p in incoming.EnumerateObject()) merged[p.Name] = p.Value.Clone(); + foreach (var s in secrets) + { + var incomingBlank = + !merged.TryGetValue(s, out var v) + || (v.ValueKind == JsonValueKind.String && string.IsNullOrEmpty(v.GetString())); + if (incomingBlank && existingDoc.RootElement.TryGetProperty(s, out var kept)) + merged[s] = kept.Clone(); + } + return JsonSerializer.Serialize(merged); + } + private static JsonElement StripSecrets(string configJson, IReadOnlySet secretFields) { using var doc = JsonDocument.Parse(configJson); diff --git a/src/VolcanicCore.Persistence/Repositories/ConnectionRepository.cs b/src/VolcanicCore.Persistence/Repositories/ConnectionRepository.cs index 4735b09..a767a1b 100644 --- a/src/VolcanicCore.Persistence/Repositories/ConnectionRepository.cs +++ b/src/VolcanicCore.Persistence/Repositories/ConnectionRepository.cs @@ -63,6 +63,22 @@ public sealed class ConnectionRepository return rows; } + public bool Update(string id, string name, string configJson) + { + using var conn = _factory.Open(); + using var cmd = conn.CreateCommand(); + cmd.CommandText = @" + UPDATE connections + SET name = $name, + config_json_encrypted = $enc + WHERE id = $id; + "; + cmd.Parameters.AddWithValue("$id", id); + cmd.Parameters.AddWithValue("$name", name); + cmd.Parameters.AddWithValue("$enc", _enc.Encrypt(configJson)); + return cmd.ExecuteNonQuery() == 1; + } + public bool Delete(string id) { using var conn = _factory.Open(); diff --git a/tests/VolcanicCore.Tests/ConnectionsEndpointsTests.cs b/tests/VolcanicCore.Tests/ConnectionsEndpointsTests.cs index 3f6b082..d5b1136 100644 --- a/tests/VolcanicCore.Tests/ConnectionsEndpointsTests.cs +++ b/tests/VolcanicCore.Tests/ConnectionsEndpointsTests.cs @@ -1,6 +1,7 @@ using System.Net; using System.Net.Http.Json; using System.Text.Json; +using Microsoft.Extensions.DependencyInjection; namespace VolcanicCore.Tests; @@ -90,5 +91,143 @@ public class ConnectionsEndpointsTests : IClassFixture Assert.DoesNotContain(get.GetProperty("config").EnumerateObject(), p => p.Name == "pass"); } + [Fact] + public async Task Put_updates_name_and_non_secret_fields() + { + var c = await LoggedIn(); + var create = await c.PostAsJsonAsync("/api/connections", new + { + providerId = "nats", name = "put-a", + config = new { url = "nats://old", user = "u", pass = "old-secret" } + }); + Assert.Equal(HttpStatusCode.Created, create.StatusCode); + var id = (await create.Content.ReadFromJsonAsync()).GetProperty("id").GetString()!; + + var put = await c.PutAsJsonAsync($"/api/connections/{id}", new + { + name = "put-a-renamed", + config = new { url = "nats://new", user = "u2", pass = "" } + }); + Assert.Equal(HttpStatusCode.OK, put.StatusCode); + var updated = await put.Content.ReadFromJsonAsync(); + Assert.Equal("put-a-renamed", updated!.Name); + Assert.Equal("nats", updated.ProviderId); + Assert.Equal("nats://new", updated.Config.GetProperty("url").GetString()); + Assert.Equal("u2", updated.Config.GetProperty("user").GetString()); + // pass is a secret field and must never come back in the response. + Assert.DoesNotContain(updated.Config.EnumerateObject(), p => p.Name == "pass"); + } + + [Fact] + public async Task Put_with_blank_secret_field_preserves_existing_value() + { + // Round-trip through NATS's OpenAsync would be the ideal check, but we + // can't stand up a real broker here. Instead: create, update with blank + // pass, then re-hydrate a raw ConnectionRow from the repo and confirm + // the stored config still has the ORIGINAL secret. + var c = await LoggedIn(); + var create = await c.PostAsJsonAsync("/api/connections", new + { + providerId = "nats", name = "keep-secret", + config = new { url = "nats://x", user = "u", pass = "original-secret" } + }); + var id = (await create.Content.ReadFromJsonAsync()).GetProperty("id").GetString()!; + + var put = await c.PutAsJsonAsync($"/api/connections/{id}", new + { + name = "keep-secret", + config = new { url = "nats://x", user = "u", pass = "" } // blank secret + }); + Assert.Equal(HttpStatusCode.OK, put.StatusCode); + + // Probe: fetch the row through the repo directly and inspect the + // decrypted stored config for the preserved secret. + using var scope = _factory.Services.CreateScope(); + var repo = scope.ServiceProvider + .GetRequiredService(); + var row = repo.Get(id); + Assert.NotNull(row); + using var stored = JsonDocument.Parse(row!.ConfigJson); + Assert.Equal("original-secret", stored.RootElement.GetProperty("pass").GetString()); + } + + [Fact] + public async Task Put_with_new_secret_field_value_overwrites_it() + { + var c = await LoggedIn(); + var create = await c.PostAsJsonAsync("/api/connections", new + { + providerId = "nats", name = "rotate-secret", + config = new { url = "nats://x", user = "u", pass = "old-secret" } + }); + var id = (await create.Content.ReadFromJsonAsync()).GetProperty("id").GetString()!; + + var put = await c.PutAsJsonAsync($"/api/connections/{id}", new + { + name = "rotate-secret", + config = new { url = "nats://x", user = "u", pass = "brand-new-secret" } + }); + Assert.Equal(HttpStatusCode.OK, put.StatusCode); + + using var scope = _factory.Services.CreateScope(); + var repo = scope.ServiceProvider + .GetRequiredService(); + var row = repo.Get(id); + using var stored = JsonDocument.Parse(row!.ConfigJson); + Assert.Equal("brand-new-secret", stored.RootElement.GetProperty("pass").GetString()); + } + + [Fact] + public async Task Put_on_unknown_id_returns_404() + { + var c = await LoggedIn(); + var put = await c.PutAsJsonAsync("/api/connections/does-not-exist", new + { + name = "x", config = new { url = "nats://x" } + }); + Assert.Equal(HttpStatusCode.NotFound, put.StatusCode); + } + + [Fact] + public async Task Put_with_duplicate_name_per_provider_returns_409() + { + var c = await LoggedIn(); + var a = await c.PostAsJsonAsync("/api/connections", new + { + providerId = "nats", name = "put-dup-a", config = new { url = "nats://a" } + }); + var b = await c.PostAsJsonAsync("/api/connections", new + { + providerId = "nats", name = "put-dup-b", config = new { url = "nats://b" } + }); + Assert.Equal(HttpStatusCode.Created, a.StatusCode); + Assert.Equal(HttpStatusCode.Created, b.StatusCode); + var bId = (await b.Content.ReadFromJsonAsync()).GetProperty("id").GetString()!; + + var put = await c.PutAsJsonAsync($"/api/connections/{bId}", new + { + name = "put-dup-a", // collides with row A + config = new { url = "nats://b" } + }); + Assert.Equal(HttpStatusCode.Conflict, put.StatusCode); + } + + [Fact] + public async Task Put_with_non_object_config_returns_400() + { + var c = await LoggedIn(); + var create = await c.PostAsJsonAsync("/api/connections", new + { + providerId = "nats", name = "put-badcfg", config = new { url = "nats://x" } + }); + var id = (await create.Content.ReadFromJsonAsync()).GetProperty("id").GetString()!; + + var put = await c.PutAsJsonAsync($"/api/connections/{id}", new + { + name = "put-badcfg", config = "not-an-object" + }); + Assert.Equal(HttpStatusCode.BadRequest, put.StatusCode); + } + private sealed record ConnectionDto(string Id, string ProviderId, string Name, JsonElement Config); }