aaronrene / knowtation public
proposal-approve-rbac-fix-security.test.mjs
189 lines 10.5 KB
Raw
sha256:700fafdd1afa490919f9515d660ca6e75456bcd5bb67513abcd8757a634c01f6 docs: record AIP-b SD-21 land (KN #308) Human 12 days ago
1 /**
2 * Security tests — proposal approve RBAC fix.
3 *
4 * Verifies that the RBAC fix does not introduce privilege escalation, secret leakage,
5 * or bypass opportunities. These are build-blocking tests.
6 *
7 * Threat model:
8 * S1. Attacker forges a JWT with role='admin' using a different secret → jwt.verify throws.
9 * S2. Attacker passes a sub that matches HUB_ADMIN_USER_IDS but JWT is invalid → override
10 * should not fire when JWT verification fails (getUserId returns null → no sub).
11 * S3. Error messages in showToast must not leak SESSION_SECRET, JWT payloads, or internal paths.
12 * S4. Bridge 401 fallback must only use the GATEWAY's SESSION_SECRET to verify — not skip verify.
13 * S5. Gateway admin override only applies to the exact sub from a verified JWT, not from query params.
14 * S6. The canApprove check in assertHostedProposalApproveDiscard must be the last gate before proxying.
15 * S7. No plaintext role claim (from an unverified source) can bypass the JWT verify step.
16 */
17
18 import { test, describe } from 'node:test';
19 import assert from 'node:assert/strict';
20 import fs from 'node:fs';
21 import path from 'node:path';
22 import { fileURLToPath } from 'node:url';
23 import jwt from 'jsonwebtoken';
24
25 const __dirname = path.dirname(fileURLToPath(import.meta.url));
26 const ROOT = path.resolve(__dirname, '..');
27
28 const REAL_SECRET = 'legit-secret-for-tests';
29 const ATTACKER_SECRET = 'attacker-secret-cannot-forge';
30
31 describe('Security: JWT forgery resistance', () => {
32 test('S1: forged JWT with wrong secret is rejected by jwt.verify', () => {
33 const forgedToken = jwt.sign({ sub: 'google:fake-admin', role: 'admin' }, ATTACKER_SECRET, { expiresIn: '1h' });
34 assert.throws(
35 () => jwt.verify(forgedToken, REAL_SECRET),
36 /invalid signature|JsonWebTokenError/,
37 'Forged JWT rejected by verify with correct secret'
38 );
39 });
40
41 test('S2: sub extraction from forged JWT fails before admin override can fire', () => {
42 // getUserId uses jwt.verify internally; a forged token means sub is null
43 const forgedToken = jwt.sign({ sub: 'google:admin-id', role: 'admin' }, ATTACKER_SECRET);
44 let sub = null;
45 try {
46 const payload = jwt.verify(forgedToken, REAL_SECRET);
47 sub = payload.sub ?? null;
48 } catch (_) {}
49 assert.equal(sub, null, 'Sub is null when JWT cannot be verified → admin override cannot fire');
50 });
51
52 test('S3: showToast error message in hub.js does not reference SESSION_SECRET, jwt, or internal paths', () => {
53 const src = fs.readFileSync(path.join(ROOT, 'web/hub/hub.js'), 'utf8');
54 const fnStart = src.indexOf('async function approveProposal');
55 const fn = src.slice(fnStart, src.indexOf('\n async function discardProposal', fnStart));
56 const catchBlock = fn.slice(fn.indexOf('} catch (e)'));
57 // The toast message is built from e.message — verify the format is safe
58 assert.ok(!catchBlock.includes('SESSION_SECRET'), 'SESSION_SECRET not in toast message template');
59 assert.ok(!catchBlock.includes('jwt.sign'), 'jwt.sign not referenced in toast template');
60 assert.ok(!catchBlock.includes('SECRET'), 'SECRET keyword not in approve catch block');
61 // Message should contain the user-facing error from the API (e.message)
62 assert.ok(catchBlock.includes('e.message'), 'toast includes API error message for user');
63 });
64
65 test('S4: bridge fallback uses once-verified bearerPayload (not JSON.parse on raw header)', () => {
66 const src = fs.readFileSync(path.join(ROOT, 'hub/gateway/server.mjs'), 'utf8');
67 const fnStart = src.indexOf('async function resolveHostedActorRole');
68 const fn = src.slice(fnStart, src.indexOf('\n}\n', fnStart) + 3);
69 // SEC-KN-3: verify once at entry; fallback must consume bearerPayload, never raw-parse.
70 // SEC-KN-P6-ROTATE: entry verify goes through the dual-secret rotation helper
71 // (primary SESSION_SECRET first; SESSION_SECRET_PREVIOUS verify-only during cutover).
72 assert.ok(
73 fn.includes('verifyJwtWithSecretRotation(token, SESSION_SECRET, SESSION_SECRET_PREVIOUS)'),
74 'Entry path verifies JWT with SESSION_SECRET (+ rotation-window previous)',
75 );
76 const fallbackBlock = fn.slice(fn.indexOf('!bridgeResolved'));
77 assert.ok(
78 fallbackBlock.includes('roleFromVerifiedAccessPayload(bearerPayload'),
79 'Bridge fallback uses verified bearerPayload (not a second unverified parse)',
80 );
81 assert.ok(!fallbackBlock.includes('JSON.parse'), 'Bridge fallback does not JSON.parse the raw header');
82 assert.ok(!fallbackBlock.includes('jwt.verify'), 'Fallback does not re-verify; it reuses bearerPayload');
83 });
84
85 test('S5: gateway admin override uses getUserId (verified sub), not query params', () => {
86 const src = fs.readFileSync(path.join(ROOT, 'hub/gateway/server.mjs'), 'utf8');
87 const fnStart = src.indexOf('async function resolveHostedActorRole');
88 const fn = src.slice(fnStart, src.indexOf('\n}\n', fnStart) + 3);
89 const overrideBlock = fn.slice(fn.indexOf('Gateway-level admin override'));
90 assert.ok(overrideBlock.includes('getUserId(req)'), 'Override uses getUserId (JWT-verified sub)');
91 assert.ok(!overrideBlock.includes('req.query'), 'Override does not use query parameters as sub');
92 assert.ok(!overrideBlock.includes('req.body'), 'Override does not use request body as sub');
93 });
94
95 test('S6: canApprove is the final gate immediately before 403 response', () => {
96 const src = fs.readFileSync(path.join(ROOT, 'hub/gateway/server.mjs'), 'utf8');
97 const fnStart = src.indexOf('async function assertHostedProposalApproveDiscard');
98 const fn = src.slice(fnStart, src.indexOf('\n}\n', fnStart) + 3);
99 const canApproveIdx = fn.indexOf('const canApprove');
100 assert.ok(canApproveIdx > 0, 'canApprove is computed');
101 // The 403 for approve specifically appears AFTER canApprove is defined.
102 // (There is also a 403 for discard before canApprove — find the approve-specific one.)
103 const approveForbiddenIdx = fn.indexOf("'FORBIDDEN'", canApproveIdx);
104 assert.ok(approveForbiddenIdx > canApproveIdx, '403 FORBIDDEN for approve follows canApprove check');
105 const returnFalseIdx = fn.indexOf('return false', approveForbiddenIdx);
106 assert.ok(returnFalseIdx > approveForbiddenIdx, 'return false after 403 prevents bypass');
107 });
108
109 test('S7: role from bridge is only accepted after roleRes.ok check', () => {
110 const src = fs.readFileSync(path.join(ROOT, 'hub/gateway/server.mjs'), 'utf8');
111 const fnStart = src.indexOf('async function resolveHostedActorRole');
112 const fn = src.slice(fnStart, src.indexOf('\n}\n', fnStart) + 3);
113 const bridgeBlock = fn.slice(fn.indexOf('else if (BRIDGE_URL'), fn.indexOf('!bridgeResolved'));
114 assert.ok(bridgeBlock.includes('roleRes.ok'), 'Bridge role is only used when roleRes.ok is true');
115 // data.role is only set inside the ok block
116 const okBlock = bridgeBlock.slice(bridgeBlock.indexOf('if (roleRes.ok)'));
117 assert.ok(okBlock.includes('data.role'), 'data.role is inside the roleRes.ok guard');
118 });
119 });
120
121 describe('Security: privilege escalation prevention', () => {
122 test('member JWT cannot become admin via bridge fallback alone', () => {
123 // When bridge returns 401, the fallback uses the JWT role.
124 // A member JWT stays member even through the fallback.
125 const token = jwt.sign({ sub: 'google:member', role: 'member' }, REAL_SECRET, { expiresIn: '1h' });
126 let role = 'member';
127 try {
128 const payload = jwt.verify(token, REAL_SECRET);
129 role = payload.role || 'member';
130 } catch (_) {}
131 assert.equal(role, 'member', 'Member JWT remains member through JWT fallback');
132 });
133
134 test('admin claim in JWT body is verified against SESSION_SECRET before use', () => {
135 // A properly signed admin JWT from the correct gateway → valid
136 const validAdminToken = jwt.sign({ sub: 'google:real-admin', role: 'admin' }, REAL_SECRET, { expiresIn: '1h' });
137 let role = 'member';
138 try {
139 const p = jwt.verify(validAdminToken, REAL_SECRET);
140 role = p.role || 'member';
141 } catch (_) {}
142 assert.equal(role, 'admin', 'Valid admin JWT verified correctly');
143
144 // Same token verified with wrong secret → throws
145 assert.throws(
146 () => jwt.verify(validAdminToken, ATTACKER_SECRET),
147 /invalid signature|JsonWebTokenError/,
148 'Admin claim rejected when verified with wrong secret'
149 );
150 });
151
152 test('gateway admin override fires only for subs in adminUserIdsSet', () => {
153 const adminSet = new Set(['google:the-real-admin']);
154 const checkOverride = (sub, currentRole) => {
155 if (sub && currentRole !== 'admin' && adminSet.has(sub)) {
156 return 'admin';
157 }
158 return currentRole;
159 };
160 assert.equal(checkOverride('google:the-real-admin', 'member'), 'admin', 'admin sub gets override');
161 assert.equal(checkOverride('google:impersonator', 'member'), 'member', 'non-admin sub gets no override');
162 assert.equal(checkOverride('', 'member'), 'member', 'empty sub gets no override');
163 assert.equal(checkOverride(null, 'member'), 'member', 'null sub gets no override');
164 assert.equal(checkOverride('google:the-real-admin', 'admin'), 'admin', 'already-admin gets no change');
165 });
166 });
167
168 describe('Security: no secrets in error surfaces', () => {
169 test('hub.js showToast does not expose raw JWT in error messages', () => {
170 const src = fs.readFileSync(path.join(ROOT, 'web/hub/hub.js'), 'utf8');
171 const fnStart = src.indexOf('async function approveProposal');
172 const fn = src.slice(fnStart, src.indexOf('\n async function discardProposal', fnStart));
173 // Pattern: showToast('Approve failed: ' + msg, true)
174 // Verify the message is built from e.message, which is a string — not the full error object
175 const toastLine = fn.slice(fn.indexOf('showToast('));
176 assert.ok(toastLine.includes('e.message || String(e)') || toastLine.includes('e.message'), 'error uses e.message string, not full error object');
177 assert.ok(!toastLine.includes('JSON.stringify(e)'), 'JSON.stringify not used on error in toast');
178 });
179
180 test('server.mjs RBAC check 403 response does not echo back sensitive request data', () => {
181 const src = fs.readFileSync(path.join(ROOT, 'hub/gateway/server.mjs'), 'utf8');
182 const fnStart = src.indexOf('async function assertHostedProposalApproveDiscard');
183 const fn = src.slice(fnStart, src.indexOf('\n}\n', fnStart) + 3);
184 const forbiddenBlock = fn.slice(fn.indexOf("'FORBIDDEN'") - 200, fn.indexOf("'FORBIDDEN'") + 200);
185 // Error message must be static — not echo req.headers or sub
186 assert.ok(!forbiddenBlock.includes('req.headers'), '403 does not echo request headers');
187 assert.ok(!forbiddenBlock.includes('req.body'), '403 does not echo request body');
188 });
189 });
File History 1 commit
sha256:700fafdd1afa490919f9515d660ca6e75456bcd5bb67513abcd8757a634c01f6 docs: record AIP-b SD-21 land (KN #308) Human 12 days ago