feat(guild): restore system-key bypass + isSystem msg path
Resurrects the x-fabric-system-key bypass + isSystem branch on POST /channels/:id/messages, dropped inca20df7when dialectic stopped broadcasting topic lifecycle events to Fabric. Re-enabling now because Fabric.OpenclawPlugin's close-sub-discussion needs to write a callback into a parent channel as a system-authored message (not as the closing host), with an optional precision wakeup so the recruitment workflow can resume immediately after an interview sub-discussion closes. Three coupled bits: 1. ApiKeyGuard pre-Bearer bypass: when x-fabric-system-key matches FABRIC_BACKEND_GUILD_COMMANDS_SYNC_KEY, set req.isSystem=true and skip the Bearer check. Intentionally reuses the existing commands sync env — same shared secret, same consumer (the OpenclawPlugin reads channels.fabric.commandsSyncKey for both paths). One less env to rotate, one less secret to manage. 2. messaging.controller POST /channels/:id/messages adds an isSystem branch (runs before the participant gate): - Looks up the channel directly (not via assertParticipant). - Persists with sentinel author 00000000-0000-0000-0000-000000000000, same UUID the old impl used. - Translates <@user.name:NAME> mentions like the regular path. - When wakeupUserId is set, delivers via emitMessageTargeted so that exactly that one recipient receives wakeup=true; everyone else gets wakeup=false. When omitted, delivers via emitMessageCreated with an empty wakeUserIds set so nobody is woken — silent system log. Two intentional differences from the985b06aoriginal: - No xType=announce restriction. The original was limited to announce because that was Dialectic's only use case; we now need this on dm / general / discuss / etc. for the sub-discussion callback. Closed channels are still rejected (409) on both paths. - The wakeupUserId field is new — old impl only ever sent silent announces. 3. DTO carries wakeupUserId? optional string. Ignored on the regular user-bearer path; load-bearing on the system path. Shared helper: extracted commands.controller's private safeEqual into src/common/safe-equal.ts so api-key.guard.ts can use the same constant- time check. Vitest spec covers equal / inequal / length-mismatch / empty cases. Existing unit tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -5,6 +5,7 @@ import {
|
||||
UnauthorizedException,
|
||||
} from '@nestjs/common';
|
||||
import { introspectGuildToken } from './center-auth.js';
|
||||
import { safeEqual } from './safe-equal.js';
|
||||
|
||||
@Injectable()
|
||||
export class ApiKeyGuard implements CanActivate {
|
||||
@@ -21,6 +22,25 @@ export class ApiKeyGuard implements CanActivate {
|
||||
return true;
|
||||
}
|
||||
|
||||
// System-key bypass: when a caller presents x-fabric-system-key
|
||||
// matching FABRIC_BACKEND_GUILD_COMMANDS_SYNC_KEY (intentionally the
|
||||
// same shared secret as x-commands-sync-key — both legitimate
|
||||
// consumers are Fabric.OpenclawPlugin), skip the Bearer requirement
|
||||
// and mark this as a system caller. Downstream handlers (e.g.
|
||||
// messaging.controller POST /channels/:id/messages) gate on
|
||||
// req.isSystem to take the system-author code path.
|
||||
//
|
||||
// Empty env → bypass disabled (no system caller ever valid; closed
|
||||
// by default). Header carries the secret as-is; we constant-time
|
||||
// compare against the env value.
|
||||
const sysExpected = (process.env.FABRIC_BACKEND_GUILD_COMMANDS_SYNC_KEY ?? '').trim();
|
||||
const sysHeader = req.headers['x-fabric-system-key'];
|
||||
const sysProvided = Array.isArray(sysHeader) ? sysHeader[0] : sysHeader;
|
||||
if (sysExpected && sysProvided && safeEqual(sysProvided, sysExpected)) {
|
||||
(req as { isSystem?: boolean }).isSystem = true;
|
||||
return true;
|
||||
}
|
||||
|
||||
const auth = req.headers['authorization'];
|
||||
const authValue = Array.isArray(auth) ? auth[0] : auth;
|
||||
let token = authValue?.startsWith('Bearer ') ? authValue.slice(7) : '';
|
||||
|
||||
25
src/common/safe-equal.spec.ts
Normal file
25
src/common/safe-equal.spec.ts
Normal file
@@ -0,0 +1,25 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { safeEqual } from './safe-equal.js';
|
||||
|
||||
describe('safeEqual', () => {
|
||||
it('returns true for identical non-empty strings', () => {
|
||||
expect(safeEqual('abc123', 'abc123')).toBe(true);
|
||||
});
|
||||
|
||||
it('returns false for different strings of same length', () => {
|
||||
expect(safeEqual('abc123', 'abc124')).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false for differing lengths', () => {
|
||||
expect(safeEqual('abc', 'abcd')).toBe(false);
|
||||
});
|
||||
|
||||
it('handles empty strings', () => {
|
||||
// both empty is technically equal — but downstream callers should
|
||||
// explicitly check for empty expected before invoking. We just
|
||||
// document the constant-time-comparison primitive's behavior.
|
||||
expect(safeEqual('', '')).toBe(true);
|
||||
expect(safeEqual('a', '')).toBe(false);
|
||||
expect(safeEqual('', 'a')).toBe(false);
|
||||
});
|
||||
});
|
||||
12
src/common/safe-equal.ts
Normal file
12
src/common/safe-equal.ts
Normal file
@@ -0,0 +1,12 @@
|
||||
import { timingSafeEqual } from 'node:crypto';
|
||||
|
||||
// Constant-time string comparison. Returns false for length mismatch (the
|
||||
// length difference itself is observable, but the per-byte loop isn't).
|
||||
// Used for shared-secret header checks (commands-sync-key, system-key,
|
||||
// etc.) to keep timing-oracle attacks off the table.
|
||||
export function safeEqual(a: string, b: string): boolean {
|
||||
const ab = Buffer.from(a);
|
||||
const bb = Buffer.from(b);
|
||||
if (ab.length !== bb.length) return false;
|
||||
return timingSafeEqual(ab, bb);
|
||||
}
|
||||
Reference in New Issue
Block a user