Skip to content

Commit 37c0520

Browse files
pgondhi987pgondhi987
authored
fix(device-pair): require pairing scope for pair command [AI] (#76377)
* fix: restrict device pairing command access * addressing review-skill * addressing review-skill * addressing codex review * address codex review feedback * addressing codex review * addressing codex review * addressing codex review * addressing codex review * docs: add changelog entry for PR merge
1 parent 30e259b commit 37c0520

11 files changed

Lines changed: 553 additions & 88 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ Docs: https://docs.openclaw.ai
4646

4747
### Fixes
4848

49+
- fix(device-pair): require pairing scope for pair command [AI]. (#76377) Thanks @pgondhi987.
4950
- fix(qqbot): keep private commands off framework surface [AI]. (#77212) Thanks @pgondhi987.
5051
- Memory/wiki: preserve representation from both corpora in `corpus=all` searches while backfilling unused result capacity, so memory hits are not starved by numerically higher wiki integer scores. Fixes #77337. Thanks @hclsys.
5152
- Telegram: clean up tool-only draft previews after assistant message boundaries so transient `Surfacing...` tool-status bubbles do not linger when no matching final preview arrives. Thanks @BunsDev.

‎extensions/device-pair/index.test.ts‎

Lines changed: 131 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,7 @@ describe("device-pair /pair qr", () => {
258258

259259
it("returns an inline QR image for webchat surfaces", async () => {
260260
const command = registerPairCommand();
261+
expect(command.requiredScopes).toEqual(["operator.pairing"]);
261262
const result = await command.handler(
262263
createCommandContext({
263264
channel: "webchat",
@@ -296,7 +297,24 @@ describe("device-pair /pair qr", () => {
296297

297298
expect(pluginApiMocks.issueDeviceBootstrapToken).not.toHaveBeenCalled();
298299
expect(result).toEqual({
299-
text: "⚠️ This command requires operator.pairing for internal gateway callers.",
300+
text: "⚠️ This command requires operator.pairing.",
301+
});
302+
});
303+
304+
it("rejects qr setup for non-gateway command surfaces without pairing scopes", async () => {
305+
const command = registerPairCommand();
306+
const result = await command.handler(
307+
createCommandContext({
308+
channel: "telegram",
309+
args: "qr",
310+
commandBody: "/pair qr",
311+
gatewayClientScopes: undefined,
312+
}),
313+
);
314+
315+
expect(pluginApiMocks.issueDeviceBootstrapToken).not.toHaveBeenCalled();
316+
expect(result).toEqual({
317+
text: "⚠️ This command requires operator.pairing.",
300318
});
301319
});
302320

@@ -433,7 +451,12 @@ describe("device-pair /pair qr", () => {
433451
runtime: createChannelRuntime(testCase.runtimeKey, testCase.sendKey, sendMessage),
434452
});
435453

436-
const result = await command.handler(createCommandContext(testCase.ctx));
454+
const result = await command.handler(
455+
createCommandContext({
456+
...testCase.ctx,
457+
gatewayClientScopes: INTERNAL_PAIRING_SCOPES,
458+
}),
459+
);
437460
const text = requireText(result);
438461

439462
expect(sendMessage).toHaveBeenCalledTimes(1);
@@ -479,6 +502,7 @@ describe("device-pair /pair qr", () => {
479502
createCommandContext({
480503
channel: "discord",
481504
senderId: "123",
505+
gatewayClientScopes: INTERNAL_PAIRING_SCOPES,
482506
}),
483507
);
484508
const text = requireText(result);
@@ -497,6 +521,7 @@ describe("device-pair /pair qr", () => {
497521
createCommandContext({
498522
channel: "msteams",
499523
senderId: "8:orgid:123",
524+
gatewayClientScopes: INTERNAL_PAIRING_SCOPES,
500525
}),
501526
);
502527
const text = requireText(result);
@@ -514,6 +539,7 @@ describe("device-pair /pair qr", () => {
514539
channel: "telegram",
515540
args: "cleanup",
516541
commandBody: "/pair cleanup",
542+
gatewayClientScopes: INTERNAL_PAIRING_SCOPES,
517543
}),
518544
);
519545

@@ -534,7 +560,7 @@ describe("device-pair /pair qr", () => {
534560

535561
expect(pluginApiMocks.clearDeviceBootstrapTokens).not.toHaveBeenCalled();
536562
expect(result).toEqual({
537-
text: "⚠️ This command requires operator.pairing for internal gateway callers.",
563+
text: "⚠️ This command requires operator.pairing.",
538564
});
539565
});
540566

@@ -551,7 +577,24 @@ describe("device-pair /pair qr", () => {
551577

552578
expect(pluginApiMocks.clearDeviceBootstrapTokens).not.toHaveBeenCalled();
553579
expect(result).toEqual({
554-
text: "⚠️ This command requires operator.pairing for internal gateway callers.",
580+
text: "⚠️ This command requires operator.pairing.",
581+
});
582+
});
583+
584+
it("rejects status for non-gateway command surfaces without pairing scopes", async () => {
585+
const command = registerPairCommand();
586+
const result = await command.handler(
587+
createCommandContext({
588+
channel: "telegram",
589+
args: "status",
590+
commandBody: "/pair status",
591+
gatewayClientScopes: undefined,
592+
}),
593+
);
594+
595+
expect(vi.mocked(listDevicePairing)).not.toHaveBeenCalled();
596+
expect(result).toEqual({
597+
text: "⚠️ This command requires operator.pairing.",
555598
});
556599
});
557600
});
@@ -578,7 +621,7 @@ describe("device-pair /pair default setup code", () => {
578621

579622
expect(pluginApiMocks.issueDeviceBootstrapToken).not.toHaveBeenCalled();
580623
expect(result).toEqual({
581-
text: "⚠️ This command requires operator.pairing for internal gateway callers.",
624+
text: "⚠️ This command requires operator.pairing.",
582625
});
583626
});
584627

@@ -595,7 +638,7 @@ describe("device-pair /pair default setup code", () => {
595638

596639
expect(pluginApiMocks.issueDeviceBootstrapToken).not.toHaveBeenCalled();
597640
expect(result).toEqual({
598-
text: "⚠️ This command requires operator.pairing for internal gateway callers.",
641+
text: "⚠️ This command requires operator.pairing.",
599642
});
600643
});
601644

@@ -612,10 +655,44 @@ describe("device-pair /pair default setup code", () => {
612655

613656
expect(pluginApiMocks.issueDeviceBootstrapToken).not.toHaveBeenCalled();
614657
expect(result).toEqual({
615-
text: "⚠️ This command requires operator.pairing for internal gateway callers.",
658+
text: "⚠️ This command requires operator.pairing.",
616659
});
617660
});
618661

662+
it("fails closed for non-gateway setup code issuance when scopes are absent", async () => {
663+
const command = registerPairCommand();
664+
const result = await command.handler(
665+
createCommandContext({
666+
channel: "telegram",
667+
args: "",
668+
commandBody: "/pair",
669+
gatewayClientScopes: undefined,
670+
}),
671+
);
672+
673+
expect(pluginApiMocks.issueDeviceBootstrapToken).not.toHaveBeenCalled();
674+
expect(result).toEqual({
675+
text: "⚠️ This command requires operator.pairing.",
676+
});
677+
});
678+
679+
it("allows command owners to issue setup codes from non-gateway command surfaces", async () => {
680+
const command = registerPairCommand();
681+
const result = await command.handler(
682+
createCommandContext({
683+
channel: "telegram",
684+
args: "",
685+
commandBody: "/pair",
686+
gatewayClientScopes: undefined,
687+
senderIsOwner: true,
688+
}),
689+
);
690+
const text = requireText(result);
691+
692+
expect(pluginApiMocks.issueDeviceBootstrapToken).toHaveBeenCalledTimes(1);
693+
expect(text).toContain("Pairing setup code generated.");
694+
});
695+
619696
it("normalizes secure bare publicUrl host ports before issuing setup codes", async () => {
620697
const command = registerPairCommand({
621698
config: {
@@ -909,7 +986,7 @@ describe("device-pair /pair approve", () => {
909986

910987
expect(vi.mocked(approveDevicePairing)).not.toHaveBeenCalled();
911988
expect(result).toEqual({
912-
text: "⚠️ This command requires operator.pairing for internal gateway callers.",
989+
text: "⚠️ This command requires operator.pairing.",
913990
});
914991
});
915992

@@ -924,7 +1001,24 @@ describe("device-pair /pair approve", () => {
9241001
expect(result).toEqual({ text: "✅ Paired Victim Phone (ios)." });
9251002
});
9261003

927-
it("does not force an empty caller scope context for external approvals", async () => {
1004+
it("rejects non-gateway approvals without pairing scopes", async () => {
1005+
const command = registerPairCommand();
1006+
const result = await command.handler(
1007+
createCommandContext({
1008+
channel: "telegram",
1009+
args: "approve latest",
1010+
commandBody: "/pair approve latest",
1011+
gatewayClientScopes: undefined,
1012+
}),
1013+
);
1014+
1015+
expect(vi.mocked(approveDevicePairing)).not.toHaveBeenCalled();
1016+
expect(result).toEqual({
1017+
text: "⚠️ This command requires operator.pairing.",
1018+
});
1019+
});
1020+
1021+
it("allows command owners to approve from non-gateway command surfaces", async () => {
9281022
mockPendingPairingList();
9291023
vi.mocked(approveDevicePairing).mockResolvedValueOnce(makeApprovedPairingResult());
9301024

@@ -935,10 +1029,32 @@ describe("device-pair /pair approve", () => {
9351029
args: "approve latest",
9361030
commandBody: "/pair approve latest",
9371031
gatewayClientScopes: undefined,
1032+
senderIsOwner: true,
9381033
}),
9391034
);
9401035

941-
expect(vi.mocked(approveDevicePairing)).toHaveBeenCalledWith("req-1");
1036+
expect(vi.mocked(approveDevicePairing)).toHaveBeenCalledWith("req-1", {
1037+
callerScopes: ["operator.pairing"],
1038+
});
1039+
expect(result).toEqual({ text: "✅ Paired Victim Phone (ios)." });
1040+
});
1041+
1042+
it("preserves gateway caller scopes for command-owner approvals", async () => {
1043+
mockPendingPairingList();
1044+
vi.mocked(approveDevicePairing).mockResolvedValueOnce(makeApprovedPairingResult());
1045+
1046+
const command = registerPairCommand();
1047+
const result = await command.handler(
1048+
createCommandContext({
1049+
channel: "telegram",
1050+
args: "approve latest",
1051+
commandBody: "/pair approve latest",
1052+
gatewayClientScopes: INTERNAL_PAIRING_SCOPES,
1053+
senderIsOwner: true,
1054+
}),
1055+
);
1056+
1057+
expectApproveCalledWithInternalPairingScopes();
9421058
expect(result).toEqual({ text: "✅ Paired Victim Phone (ios)." });
9431059
});
9441060

@@ -957,7 +1073,7 @@ describe("device-pair /pair approve", () => {
9571073

9581074
expect(vi.mocked(approveDevicePairing)).not.toHaveBeenCalled();
9591075
expect(result).toEqual({
960-
text: "⚠️ This command requires operator.pairing for internal gateway callers.",
1076+
text: "⚠️ This command requires operator.pairing.",
9611077
});
9621078
});
9631079

@@ -978,36 +1094,21 @@ describe("device-pair /pair approve", () => {
9781094
});
9791095
});
9801096

981-
it("preserves approvals for non-gateway command surfaces", async () => {
1097+
it("approves from command surfaces that carry pairing scopes", async () => {
9821098
mockPendingPairingList();
983-
vi.mocked(approveDevicePairing).mockResolvedValueOnce(
984-
makeApprovedPairingResult({
985-
device: {
986-
scopes: ["operator.admin"],
987-
approvedScopes: ["operator.admin"],
988-
tokens: {
989-
operator: {
990-
token: "token-1",
991-
role: "operator",
992-
scopes: ["operator.admin"],
993-
createdAtMs: Date.now(),
994-
},
995-
},
996-
},
997-
}),
998-
);
1099+
vi.mocked(approveDevicePairing).mockResolvedValueOnce(makeApprovedPairingResult());
9991100

10001101
const command = registerPairCommand();
10011102
const result = await command.handler(
10021103
createCommandContext({
10031104
channel: "telegram",
10041105
args: "approve latest",
10051106
commandBody: "/pair approve latest",
1006-
gatewayClientScopes: undefined,
1107+
gatewayClientScopes: INTERNAL_PAIRING_SCOPES,
10071108
}),
10081109
);
10091110

1010-
expect(vi.mocked(approveDevicePairing)).toHaveBeenCalledWith("req-1");
1111+
expectApproveCalledWithInternalPairingScopes();
10111112
expect(result).toEqual({ text: "✅ Paired Victim Phone (ios)." });
10121113
});
10131114
});

‎extensions/device-pair/index.ts‎

Lines changed: 6 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -579,20 +579,6 @@ function resolveQrReplyTarget(ctx: QrCommandContext): string {
579579
);
580580
}
581581

582-
const PAIR_SETUP_NON_ISSUING_ACTIONS = new Set([
583-
"approve",
584-
"cleanup",
585-
"clear",
586-
"notify",
587-
"pending",
588-
"revoke",
589-
"status",
590-
]);
591-
592-
function issuesPairSetupCode(action: string): boolean {
593-
return !action || action === "qr" || !PAIR_SETUP_NON_ISSUING_ACTIONS.has(action);
594-
}
595-
596582
async function issueSetupPayload(url: string): Promise<SetupPayload> {
597583
const { issueDeviceBootstrapToken, PAIRING_SETUP_BOOTSTRAP_PROFILE } =
598584
await loadDevicePairApiModule();
@@ -661,6 +647,7 @@ export default definePluginEntry({
661647
name: "pair",
662648
description: "Generate setup codes and approve device pairing requests.",
663649
acceptsArgs: true,
650+
requiredScopes: ["operator.pairing"],
664651
handler: async (ctx) => {
665652
const args = normalizeOptionalString(ctx.args) ?? "";
666653
const tokens = args.split(/\s+/).filter(Boolean);
@@ -673,13 +660,18 @@ export default definePluginEntry({
673660
const authState = resolvePairingCommandAuthState({
674661
channel: ctx.channel,
675662
gatewayClientScopes,
663+
senderIsOwner: ctx.senderIsOwner,
676664
});
677665
api.logger.info?.(
678666
`device-pair: /pair invoked channel=${ctx.channel} sender=${ctx.senderId ?? "unknown"} action=${
679667
action || "new"
680668
}`,
681669
);
682670

671+
if (authState.isMissingPairingPrivilege) {
672+
return buildMissingPairingScopeReply();
673+
}
674+
683675
if (action === "status" || action === "pending") {
684676
const [{ listDevicePairing }, { formatPendingRequests }] = await Promise.all([
685677
loadDevicePairApiModule(),
@@ -700,9 +692,6 @@ export default definePluginEntry({
700692
}
701693

702694
if (action === "approve") {
703-
if (authState.isMissingInternalPairingPrivilege) {
704-
return buildMissingPairingScopeReply();
705-
}
706695
const [
707696
{ listDevicePairing },
708697
{ approvePendingPairingRequest, selectPendingApprovalRequest },
@@ -726,9 +715,6 @@ export default definePluginEntry({
726715
}
727716

728717
if (action === "cleanup" || action === "clear" || action === "revoke") {
729-
if (authState.isMissingInternalPairingPrivilege) {
730-
return buildMissingPairingScopeReply();
731-
}
732718
const { clearDeviceBootstrapTokens } = await loadDevicePairApiModule();
733719
const cleared = await clearDeviceBootstrapTokens();
734720
return {
@@ -743,10 +729,6 @@ export default definePluginEntry({
743729
if (authLabelResult.error) {
744730
return { text: `Error: ${authLabelResult.error}` };
745731
}
746-
if (issuesPairSetupCode(action) && authState.isMissingInternalPairingPrivilege) {
747-
return buildMissingPairingScopeReply();
748-
}
749-
750732
const urlResult = await resolveMobilePairingGatewayUrl(api);
751733
if (!urlResult.url) {
752734
return { text: `Error: ${urlResult.error ?? "Gateway URL unavailable."}` };

0 commit comments

Comments
 (0)