Public: anyone can read this; the crew can change it.

GET /api/v1/secrets/:scope carries an ETag and answers a repeat with 304 until a secret in that scope is set or removed #9211

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

mason/forge:mason/9200-get-api-v1-secrets-scope-carries-an-etag → main 2 commits fork

  • Merged. mason/9200-get-api-v1-secrets-scope-carries-an-etag was merged into main as a0c131d.

Review

Sign in to review this change.
2 files changed +170 −1
src/http/api/secrets.ts +10 −1
@@ -7,6 +7,7 @@
77 import { listSecrets, putSecret, removeSecret } from "../../actions/secrets.ts";
88 import { notePhase, takeSessionTime, timePhase } from "../../core/route_timing.ts";
99 import type { Handler } from "../context.ts";
10 import { holds, weakEtag } from "../etag.ts";
1011 import { body, json, requireScope, respondAccess, str } from "../respond.ts";
1112 import type { Router } from "../router.ts";
1213
@@ -28,7 +29,15 @@ export function secretRoutes(router: Router<Handler>): void {
2829 ),
2930 );
3031 if (!listed.ok) return respondAccess(listed.error, 422);
31 return timePhase(ctx.request, "render", () => json({ items: listed.value, next_cursor: null }));
32 // The tag is of the exact bytes sent: names and timestamps, never a value. A secret set, replaced
33 // or removed in this scope changes the body; a denial or unknown scope returned above carries none.
34 return timePhase(ctx.request, "render", () => {
35 const text = JSON.stringify({ items: listed.value, next_cursor: null });
36 const etag = weakEtag(text);
37 const headers = { etag, "cache-control": "private, no-cache", vary: "authorization, cookie" };
38 if (holds(ctx.request, etag)) return new Response(null, { status: 304, headers });
39 return new Response(text, { headers: { "content-type": "application/json", ...headers } });
40 });
3241 });
3342
3443 router.add("PUT", "/api/v1/secrets/:scope/:name", async (ctx) => {
test/secrets-etag.test.ts added +160
@@ -0,0 +1,160 @@
1 /** GET /api/v1/secrets/:scope carries a weak ETag and answers a repeat with 304 until a secret in that scope moves (#9200). */
2 import { afterAll, beforeAll, expect, test } from "bun:test";
3 import { mkdtemp, rm } from "node:fs/promises";
4 import { tmpdir } from "node:os";
5 import { join } from "node:path";
6 import { putSecret, removeSecret } from "../src/actions/secrets.ts";
7 import { createRepo } from "../src/actions/repos.ts";
8 import { createApp, type App } from "../src/app.ts";
9 import { createToken } from "../src/core/tokens.ts";
10 import { createUser } from "../src/core/users.ts";
11 import { serve, type Servers } from "../src/http/server.ts";
12 import { quietLogs } from "./helpers/globals.ts";
13
14 quietLogs();
15
16 let dataDir = "";
17 let app: App, servers: Servers;
18 let alice: any;
19 let adminTok = "", readTok = "", otherTok = "";
20 const get = (path: string, tok: string, headers: Record<string, string> = {}) =>
21 fetch(`${servers.url}/api/v1/secrets/${path}`, {
22 headers: { authorization: `Bearer ${tok}`, ...headers },
23 });
24 const tagOf = async (path: string) => {
25 const res = await get(path, adminTok);
26 expect(res.status).toBe(200);
27 await res.text();
28 return res.headers.get("etag")!;
29 };
30 async function changed(path: string, before: string, what: string): Promise<string> {
31 const res = await get(path, adminTok, { "if-none-match": before });
32 expect(res.status, what).toBe(200);
33 await res.text();
34 const tag = res.headers.get("etag")!;
35 expect(tag, what).not.toBe(before);
36 return tag;
37 }
38 async function still(path: string, tag: string, what: string): Promise<void> {
39 const res = await get(path, adminTok, { "if-none-match": tag });
40 expect(res.status, what).toBe(304);
41 await res.text();
42 }
43 // updated_at has millisecond resolution; a replace in the same millisecond would be the same body.
44 const tick = () => Bun.sleep(5);
45
46 beforeAll(async () => {
47 dataDir = await mkdtemp(join(tmpdir(), "forge-secrets-etag-"));
48 app = createApp({ dataDir, port: 0 });
49 servers = await serve(app);
50 const mk = async (username: string, first: boolean) => {
51 const made = await createUser(app.db, {
52 origin: "seed",
53 trusted: true,
54 username,
55 email: `${username}@forge.local`,
56 password: "a-good-passphrase",
57 adminIfFirst: first,
58 });
59 if (!made.ok) throw new Error(String(made.error));
60 return made.value;
61 };
62 alice = await mk("alice", true);
63 const bob = await mk("bob", false);
64 for (const name of ["a", "b"]) {
65 const r = await createRepo(app, alice, { name, visibility: "public" });
66 if (!r.ok) throw new Error("repo");
67 }
68 adminTok = createToken(app.db, alice.id, "t", "admin").token;
69 readTok = createToken(app.db, alice.id, "r", "ro").token;
70 otherTok = createToken(app.db, bob.id, "o", "admin").token;
71 });
72 afterAll(async () => {
73 servers.stop();
74 app.close();
75 await rm(dataDir, { recursive: true, force: true });
76 });
77
78 test("200 carries a weak tag, private no-cache and vary; a repeat is a bodiless 304 with the floor headers", async () => {
79 expect(putSecret(app, alice, "forge", "", "TOKEN", "s3cr3t-value").ok).toBe(true);
80 const res = await get("forge", adminTok);
81 expect(res.status).toBe(200);
82 const tag = res.headers.get("etag")!;
83 expect(tag).toMatch(/^W\//);
84 expect(res.headers.get("cache-control")).toBe("private, no-cache");
85 expect(res.headers.get("vary")).toContain("authorization, cookie");
86 expect(res.headers.get("content-type")).toBe("application/json");
87 const text = await res.text();
88 expect(text).not.toContain("s3cr3t-value");
89 const parsed = JSON.parse(text) as { items: { name: string }[]; next_cursor: null };
90 expect(parsed.items.map((i) => i.name)).toEqual(["TOKEN"]);
91 expect(parsed.next_cursor).toBeNull();
92 const again = await get("forge", adminTok, { "if-none-match": tag });
93 expect(again.status).toBe(304);
94 expect(again.headers.get("etag")).toBe(tag);
95 expect(again.headers.get("cache-control")).toBe("private, no-cache");
96 expect(again.headers.get("vary")).toContain("authorization, cookie");
97 expect(again.headers.get("x-content-type-options")).toBe("nosniff");
98 expect(again.headers.get("content-security-policy")).not.toBeNull();
99 expect(again.headers.get("referrer-policy")).not.toBeNull();
100 expect(await again.text()).toBe("");
101 });
102
103 test("a secret set, replaced and removed in the scope each move the tag", async () => {
104 let tag = await tagOf("forge");
105 await tick();
106 expect(putSecret(app, alice, "forge", "", "OTHER", "v1").ok).toBe(true);
107 tag = await changed("forge", tag, "set");
108 await tick();
109 expect(putSecret(app, alice, "forge", "", "OTHER", "v2").ok).toBe(true);
110 tag = await changed("forge", tag, "replaced");
111 expect(removeSecret(app, alice, "forge", "", "OTHER").ok).toBe(true);
112 await changed("forge", tag, "removed");
113 });
114
115 test("a change in another scope or ref leaves the tag; a change in this ref moves it", async () => {
116 expect(putSecret(app, alice, "repo", "alice/a", "KEY", "x").ok).toBe(true);
117 const forge = await tagOf("forge");
118 const a = await tagOf("repo?ref=alice/a");
119 await tick();
120 expect(putSecret(app, alice, "repo", "alice/b", "KEY", "y").ok).toBe(true);
121 await still("forge", forge, "another scope");
122 await still("repo?ref=alice/a", a, "another ref");
123 await tick();
124 expect(putSecret(app, alice, "repo", "alice/a", "KEY", "z").ok).toBe(true);
125 await changed("repo?ref=alice/a", a, "this ref");
126 await still("forge", forge, "repo change leaves forge");
127 });
128
129 test("viewers shown different bodies never share a tag", async () => {
130 const a = await get("repo?ref=alice/a", adminTok);
131 const b = await get("repo?ref=alice/b", adminTok);
132 const aText = await a.text();
133 const bText = await b.text();
134 expect(aText).not.toBe(bText);
135 const aTag = a.headers.get("etag")!;
136 expect(aTag).not.toBe(b.headers.get("etag"));
137 const replay = await get("repo?ref=alice/b", adminTok, { "if-none-match": aTag });
138 expect(replay.status).toBe(200);
139 expect(await replay.text()).toBe(bText);
140 });
141
142 test("a short token, a non-admin and an unknown scope answer as before, with no ETag, even with If-None-Match: *", async () => {
143 const cases: [string, string][] = [
144 ["forge", readTok],
145 ["forge", otherTok],
146 ["nope", adminTok],
147 ["repo?ref=alice/missing", adminTok],
148 ];
149 for (const [path, tok] of cases) {
150 const plain = await get(path, tok);
151 const plainText = await plain.text();
152 const starred = await get(path, tok, { "if-none-match": "*" });
153 const starredText = await starred.text();
154 expect(plain.status, path).toBeGreaterThanOrEqual(400);
155 expect(starred.status, path).toBe(plain.status);
156 expect(starredText, path).toBe(plainText);
157 expect(plain.headers.get("etag"), path).toBeNull();
158 expect(starred.headers.get("etag"), path).toBeNull();
159 }
160 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs