aaronrene / knowtation public
proposal-approve-rbac-fix-security.test.mjs
179 lines 9.9 KB
Raw
sha256:0d530f9ef27b8b75547d1db7701a74bc77b77aa8f3d7fa3a8672cf2af36e63bb reconcile: import GitHub-direct RBAC/OAuth/companion and ho… Human minor ⚠ breaking 7 hours 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 jwt.verify (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 // The fallback must verify the JWT, not just parse it
70 const fallbackBlock = fn.slice(fn.indexOf('!bridgeResolved'));
71 assert.ok(fallbackBlock.includes('jwt.verify'), 'Bridge fallback uses jwt.verify (not raw parse)');
72 assert.ok(!fallbackBlock.includes('JSON.parse'), 'Bridge fallback does not JSON.parse the raw header');
73 });
74
75 test('S5: gateway admin override uses getUserId (verified sub), not query params', () => {
76 const src = fs.readFileSync(path.join(ROOT, 'hub/gateway/server.mjs'), 'utf8');
77 const fnStart = src.indexOf('async function resolveHostedActorRole');
78 const fn = src.slice(fnStart, src.indexOf('\n}\n', fnStart) + 3);
79 const overrideBlock = fn.slice(fn.indexOf('Gateway-level admin override'));
80 assert.ok(overrideBlock.includes('getUserId(req)'), 'Override uses getUserId (JWT-verified sub)');
81 assert.ok(!overrideBlock.includes('req.query'), 'Override does not use query parameters as sub');
82 assert.ok(!overrideBlock.includes('req.body'), 'Override does not use request body as sub');
83 });
84
85 test('S6: canApprove is the final gate immediately before 403 response', () => {
86 const src = fs.readFileSync(path.join(ROOT, 'hub/gateway/server.mjs'), 'utf8');
87 const fnStart = src.indexOf('async function assertHostedProposalApproveDiscard');
88 const fn = src.slice(fnStart, src.indexOf('\n}\n', fnStart) + 3);
89 const canApproveIdx = fn.indexOf('const canApprove');
90 assert.ok(canApproveIdx > 0, 'canApprove is computed');
91 // The 403 for approve specifically appears AFTER canApprove is defined.
92 // (There is also a 403 for discard before canApprove — find the approve-specific one.)
93 const approveForbiddenIdx = fn.indexOf("'FORBIDDEN'", canApproveIdx);
94 assert.ok(approveForbiddenIdx > canApproveIdx, '403 FORBIDDEN for approve follows canApprove check');
95 const returnFalseIdx = fn.indexOf('return false', approveForbiddenIdx);
96 assert.ok(returnFalseIdx > approveForbiddenIdx, 'return false after 403 prevents bypass');
97 });
98
99 test('S7: role from bridge is only accepted after roleRes.ok check', () => {
100 const src = fs.readFileSync(path.join(ROOT, 'hub/gateway/server.mjs'), 'utf8');
101 const fnStart = src.indexOf('async function resolveHostedActorRole');
102 const fn = src.slice(fnStart, src.indexOf('\n}\n', fnStart) + 3);
103 const bridgeBlock = fn.slice(fn.indexOf('else if (BRIDGE_URL'), fn.indexOf('!bridgeResolved'));
104 assert.ok(bridgeBlock.includes('roleRes.ok'), 'Bridge role is only used when roleRes.ok is true');
105 // data.role is only set inside the ok block
106 const okBlock = bridgeBlock.slice(bridgeBlock.indexOf('if (roleRes.ok)'));
107 assert.ok(okBlock.includes('data.role'), 'data.role is inside the roleRes.ok guard');
108 });
109 });
110
111 describe('Security: privilege escalation prevention', () => {
112 test('member JWT cannot become admin via bridge fallback alone', () => {
113 // When bridge returns 401, the fallback uses the JWT role.
114 // A member JWT stays member even through the fallback.
115 const token = jwt.sign({ sub: 'google:member', role: 'member' }, REAL_SECRET, { expiresIn: '1h' });
116 let role = 'member';
117 try {
118 const payload = jwt.verify(token, REAL_SECRET);
119 role = payload.role || 'member';
120 } catch (_) {}
121 assert.equal(role, 'member', 'Member JWT remains member through JWT fallback');
122 });
123
124 test('admin claim in JWT body is verified against SESSION_SECRET before use', () => {
125 // A properly signed admin JWT from the correct gateway → valid
126 const validAdminToken = jwt.sign({ sub: 'google:real-admin', role: 'admin' }, REAL_SECRET, { expiresIn: '1h' });
127 let role = 'member';
128 try {
129 const p = jwt.verify(validAdminToken, REAL_SECRET);
130 role = p.role || 'member';
131 } catch (_) {}
132 assert.equal(role, 'admin', 'Valid admin JWT verified correctly');
133
134 // Same token verified with wrong secret → throws
135 assert.throws(
136 () => jwt.verify(validAdminToken, ATTACKER_SECRET),
137 /invalid signature|JsonWebTokenError/,
138 'Admin claim rejected when verified with wrong secret'
139 );
140 });
141
142 test('gateway admin override fires only for subs in adminUserIdsSet', () => {
143 const adminSet = new Set(['google:the-real-admin']);
144 const checkOverride = (sub, currentRole) => {
145 if (sub && currentRole !== 'admin' && adminSet.has(sub)) {
146 return 'admin';
147 }
148 return currentRole;
149 };
150 assert.equal(checkOverride('google:the-real-admin', 'member'), 'admin', 'admin sub gets override');
151 assert.equal(checkOverride('google:impersonator', 'member'), 'member', 'non-admin sub gets no override');
152 assert.equal(checkOverride('', 'member'), 'member', 'empty sub gets no override');
153 assert.equal(checkOverride(null, 'member'), 'member', 'null sub gets no override');
154 assert.equal(checkOverride('google:the-real-admin', 'admin'), 'admin', 'already-admin gets no change');
155 });
156 });
157
158 describe('Security: no secrets in error surfaces', () => {
159 test('hub.js showToast does not expose raw JWT in error messages', () => {
160 const src = fs.readFileSync(path.join(ROOT, 'web/hub/hub.js'), 'utf8');
161 const fnStart = src.indexOf('async function approveProposal');
162 const fn = src.slice(fnStart, src.indexOf('\n async function discardProposal', fnStart));
163 // Pattern: showToast('Approve failed: ' + msg, true)
164 // Verify the message is built from e.message, which is a string — not the full error object
165 const toastLine = fn.slice(fn.indexOf('showToast('));
166 assert.ok(toastLine.includes('e.message || String(e)') || toastLine.includes('e.message'), 'error uses e.message string, not full error object');
167 assert.ok(!toastLine.includes('JSON.stringify(e)'), 'JSON.stringify not used on error in toast');
168 });
169
170 test('server.mjs RBAC check 403 response does not echo back sensitive request data', () => {
171 const src = fs.readFileSync(path.join(ROOT, 'hub/gateway/server.mjs'), 'utf8');
172 const fnStart = src.indexOf('async function assertHostedProposalApproveDiscard');
173 const fn = src.slice(fnStart, src.indexOf('\n}\n', fnStart) + 3);
174 const forbiddenBlock = fn.slice(fn.indexOf("'FORBIDDEN'") - 200, fn.indexOf("'FORBIDDEN'") + 200);
175 // Error message must be static — not echo req.headers or sub
176 assert.ok(!forbiddenBlock.includes('req.headers'), '403 does not echo request headers');
177 assert.ok(!forbiddenBlock.includes('req.body'), '403 does not echo request body');
178 });
179 });
File History 1 commit
sha256:0d530f9ef27b8b75547d1db7701a74bc77b77aa8f3d7fa3a8672cf2af36e63bb reconcile: import GitHub-direct RBAC/OAuth/companion and ho… Human minor ⚠ 7 hours ago