Skip to content

Commit 03c48d9

Browse files
sandl99sandl99
committed
fix(messaging): harden Teams onboarding plan
1 parent 353beba commit 03c48d9

14 files changed

Lines changed: 258 additions & 43 deletions

‎agents/hermes/policy-additions.yaml‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -300,9 +300,6 @@ network_policies:
300300
rules:
301301
- allow: { method: GET, path: "/**" }
302302
- allow: { method: POST, path: "/**" }
303-
- allow: { method: PATCH, path: "/**" }
304-
- allow: { method: PUT, path: "/**" }
305-
- allow: { method: DELETE, path: "/**" }
306303
- host: teams.microsoft.com
307304
port: 443
308305
protocol: rest

‎nemoclaw-blueprint/policies/presets/teams.yaml‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,6 @@ network_policies:
5555
rules:
5656
- allow: { method: GET, path: "/**" }
5757
- allow: { method: POST, path: "/**" }
58-
- allow: { method: PATCH, path: "/**" }
59-
- allow: { method: PUT, path: "/**" }
60-
- allow: { method: DELETE, path: "/**" }
6158
# Read-only Teams/Office media surfaces referenced by Teams messages.
6259
- host: teams.microsoft.com
6360
port: 443

‎src/lib/messaging/applier/build/messaging-build-applier.mts‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,12 @@ type HermesUvPackageInstall = {
112112
readonly spec: string;
113113
};
114114

115+
function isPinnedHermesUvPackageSpec(spec: string): boolean {
116+
return /^[A-Za-z0-9][A-Za-z0-9_.-]*(?:\[[A-Za-z0-9][A-Za-z0-9_.-]*(?:,[A-Za-z0-9][A-Za-z0-9_.-]*)*\])?==[A-Za-z0-9][A-Za-z0-9_.!+~-]*$/.test(
117+
spec,
118+
);
119+
}
120+
115121
export class MessagingBuildApplierError extends Error {}
116122

117123
export const DEFAULT_MESSAGING_RUNTIME_PLAN_PATH =
@@ -885,13 +891,13 @@ function readHermesUvPipPackageInstall(
885891
}
886892
if (typeof install.spec !== "string" || install.spec.trim().length === 0) {
887893
throw new MessagingBuildApplierError(
888-
`Messaging package-install output ${outputId} must include a Hermes Python package name`,
894+
`Messaging package-install output ${outputId} must include a Hermes Python package spec`,
889895
);
890896
}
891897
const spec = install.spec.trim();
892-
if (!/^[A-Za-z0-9_.-]+$/.test(spec)) {
898+
if (!isPinnedHermesUvPackageSpec(spec)) {
893899
throw new MessagingBuildApplierError(
894-
`Messaging package-install output ${outputId} has an unsafe Hermes Python package name`,
900+
`Messaging package-install output ${outputId} must use a safe exact-pinned Hermes Python package spec`,
895901
);
896902
}
897903
return { spec };
@@ -1389,6 +1395,7 @@ function installHermesMessagingUvPackages(plan: MessagingBuildPlan | null, env:
13891395
"--python",
13901396
"/opt/hermes/.venv/bin/python",
13911397
"--no-cache",
1398+
"--",
13921399
...selectedPackages,
13931400
],
13941401
env,

‎src/lib/messaging/channels/manifests.test.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -680,6 +680,7 @@ describe("built-in channel manifests", () => {
680680
expect(tenantId.envAliases).toEqual(["TEAMS_TENANT_ID"]);
681681
expect(allowedUsers.envKey).toBe("TEAMS_ALLOWED_USERS");
682682
expect(allowedUsers.envAliases).toEqual(["MSTEAMS_ALLOWED_USERS"]);
683+
expect(allowedUsers.required).toBe(true);
683684
expect(webhookPort.envKey).toBe("MSTEAMS_PORT");
684685
expect(webhookPort.envAliases).toEqual(["TEAMS_PORT"]);
685686
expect(webhookPort).toMatchObject({ kind: "config", defaultValue: "3978" });
@@ -746,6 +747,7 @@ describe("built-in channel manifests", () => {
746747
{
747748
id: "allowedUsers",
748749
kind: "config",
750+
required: true,
749751
},
750752
{
751753
id: "webhookPort",
@@ -770,14 +772,14 @@ describe("built-in channel manifests", () => {
770772
id: "hermesTeamsAppsPackage",
771773
agent: "hermes",
772774
manager: "hermes-uv-pip",
773-
spec: "microsoft-teams-apps",
775+
spec: "microsoft-teams-apps==2.0.13.4",
774776
required: true,
775777
});
776778
expect(teamsManifest.agentPackages).toContainEqual({
777779
id: "hermesAiohttpPackage",
778780
agent: "hermes",
779781
manager: "hermes-uv-pip",
780-
spec: "aiohttp",
782+
spec: "aiohttp==3.14.1",
781783
required: true,
782784
});
783785
expect(teamsManifest.state).toEqual({

‎src/lib/messaging/channels/metadata.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,14 +159,14 @@ describe("built-in messaging channel metadata", () => {
159159
packageId: "hermesTeamsAppsPackage",
160160
agents: ["hermes"],
161161
manager: "hermes-uv-pip",
162-
spec: "microsoft-teams-apps",
162+
spec: "microsoft-teams-apps==2.0.13.4",
163163
},
164164
{
165165
channelId: "teams",
166166
packageId: "hermesAiohttpPackage",
167167
agents: ["hermes"],
168168
manager: "hermes-uv-pip",
169-
spec: "aiohttp",
169+
spec: "aiohttp==3.14.1",
170170
},
171171
]);
172172
});

‎src/lib/messaging/channels/teams/manifest.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,13 @@ export const teamsManifest = {
5555
{
5656
id: "allowedUsers",
5757
kind: "config",
58-
required: false,
58+
required: true,
5959
envKey: "TEAMS_ALLOWED_USERS",
6060
envAliases: ["MSTEAMS_ALLOWED_USERS"],
6161
statePath: "allowedIds.teams",
6262
prompt: {
6363
label: "Microsoft Teams AAD Object IDs (comma-separated allowlist)",
6464
help: "Recommended: run `teams status --verbose` and enter the Azure AD object IDs allowed to use the bot.",
65-
emptyValueMessage: "bot will rely on agent-side pairing or upstream defaults",
6665
},
6766
},
6867
{
@@ -194,14 +193,14 @@ export const teamsManifest = {
194193
id: "hermesTeamsAppsPackage",
195194
agent: "hermes",
196195
manager: "hermes-uv-pip",
197-
spec: "microsoft-teams-apps",
196+
spec: "microsoft-teams-apps==2.0.13.4",
198197
required: true,
199198
},
200199
{
201200
id: "hermesAiohttpPackage",
202201
agent: "hermes",
203202
manager: "hermes-uv-pip",
204-
spec: "aiohttp",
203+
spec: "aiohttp==3.14.1",
205204
required: true,
206205
},
207206
],
@@ -283,6 +282,7 @@ export const teamsManifest = {
283282
{
284283
id: "allowedUsers",
285284
kind: "config",
285+
required: true,
286286
},
287287
{
288288
id: "webhookPort",

‎src/lib/messaging/compiler/manifest-compiler.test.ts‎

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,7 @@ describe("ManifestCompiler", () => {
419419
required: true,
420420
value: {
421421
manager: "hermes-uv-pip",
422-
spec: "microsoft-teams-apps",
422+
spec: "microsoft-teams-apps==2.0.13.4",
423423
},
424424
},
425425
{
@@ -429,7 +429,7 @@ describe("ManifestCompiler", () => {
429429
required: true,
430430
value: {
431431
manager: "hermes-uv-pip",
432-
spec: "aiohttp",
432+
spec: "aiohttp==3.14.1",
433433
},
434434
},
435435
]);
@@ -507,6 +507,7 @@ describe("ManifestCompiler", () => {
507507
{
508508
MSTEAMS_APP_ID: "test-teams-app-id",
509509
MSTEAMS_TENANT_ID: "test-teams-tenant-id",
510+
TEAMS_ALLOWED_USERS: "00000000-0000-0000-0000-000000000001",
510511
MSTEAMS_PORT: undefined,
511512
TEAMS_PORT: undefined,
512513
TEAMS_REQUIRE_MENTION: undefined,
@@ -550,6 +551,34 @@ describe("ManifestCompiler", () => {
550551
expect(JSON.stringify(plan.agentRender)).toContain('"requireMention":true');
551552
});
552553

554+
it("keeps Microsoft Teams disabled when no explicit user allowlist is provided", async () => {
555+
const plan = await withEnv(
556+
{
557+
MSTEAMS_APP_ID: "test-teams-app-id",
558+
MSTEAMS_TENANT_ID: "test-teams-tenant-id",
559+
TEAMS_ALLOWED_USERS: undefined,
560+
},
561+
() =>
562+
compiler().compile({
563+
sandboxName: "demo",
564+
agent: "openclaw",
565+
workflow: "rebuild",
566+
isInteractive: false,
567+
configuredChannels: ["teams"],
568+
credentialAvailability: {
569+
MSTEAMS_APP_PASSWORD: true,
570+
},
571+
}),
572+
);
573+
574+
expect(plan.channels.find((channel) => channel.channelId === "teams")).toMatchObject({
575+
active: false,
576+
configured: true,
577+
disabled: true,
578+
});
579+
expect(JSON.stringify(plan.agentRender)).not.toContain("channels.msteams");
580+
});
581+
553582
it("uses the configured Microsoft Teams webhook port for host forwarding", async () => {
554583
const plan = await withEnv(
555584
{
@@ -681,8 +710,9 @@ describe("ManifestCompiler", () => {
681710
});
682711
expect(plan.disabledChannels).toEqual(["wechat"]);
683712
expect(plan.networkPolicy.entries.map((entry) => entry.channelId)).toEqual(["wechat"]);
684-
expect(plan.agentRender.map((render) => render.channelId)).toEqual(["wechat", "wechat"]);
685-
expect(plan.healthChecks.map((entry) => entry.channelId)).toEqual(["wechat"]);
713+
expect(plan.agentRender).toEqual([]);
714+
expect(plan.buildSteps).toEqual([]);
715+
expect(plan.healthChecks).toEqual([]);
686716
});
687717

688718
it("runs enrollment hooks before returning the final channel input plan", async () => {
@@ -762,7 +792,7 @@ describe("ManifestCompiler", () => {
762792
expect(plan.runtimeSetup).toEqual({ nodePreloads: [], envAliases: [], secretScans: [] });
763793
expect(plan.credentialBindings.map((binding) => binding.channelId)).toEqual(["telegram"]);
764794
expect(plan.networkPolicy.entries.map((entry) => entry.channelId)).toEqual(["telegram"]);
765-
expect(plan.agentRender.map((render) => render.channelId)).toEqual(["telegram", "telegram"]);
795+
expect(plan.agentRender).toEqual([]);
766796
expect(plan.buildSteps).toEqual([]);
767797
expect(plan.stateUpdates.map((entry) => entry.channelId)).toEqual([
768798
"telegram",
@@ -771,7 +801,7 @@ describe("ManifestCompiler", () => {
771801
"telegram",
772802
"telegram",
773803
]);
774-
expect(plan.healthChecks.map((entry) => entry.channelId)).toEqual(["telegram"]);
804+
expect(plan.healthChecks).toEqual([]);
775805
});
776806

777807
it("runs non-interactive enrollment hooks to validate and feed reachability checks", async () => {
@@ -1179,7 +1209,7 @@ describe("ManifestCompiler", () => {
11791209
] satisfies Array<keyof SandboxMessagingPlan>);
11801210
});
11811211

1182-
it("records disabled configured channels and leaves applier exclusion to disabledChannels", async () => {
1212+
it("records disabled configured channels without side-effect plans", async () => {
11831213
const plan = await compiler().compile({
11841214
sandboxName: "demo",
11851215
agent: "openclaw",
@@ -1199,11 +1229,7 @@ describe("ManifestCompiler", () => {
11991229
expect(plan.disabledChannels).toEqual(["telegram"]);
12001230
expect(plan.credentialBindings.map((binding) => binding.channelId)).toEqual(["telegram"]);
12011231
expect(plan.networkPolicy.entries.map((entry) => entry.channelId)).toEqual(["telegram"]);
1202-
expect(plan.agentRender.map((render) => render.channelId)).toEqual([
1203-
"telegram",
1204-
"telegram",
1205-
"telegram",
1206-
]);
1232+
expect(plan.agentRender).toEqual([]);
12071233
expect(plan.buildSteps).toEqual([]);
12081234
expect(plan.stateUpdates.map((entry) => entry.channelId)).toEqual([
12091235
"telegram",
@@ -1212,7 +1238,7 @@ describe("ManifestCompiler", () => {
12121238
"telegram",
12131239
"telegram",
12141240
]);
1215-
expect(plan.healthChecks.map((entry) => entry.channelId)).toEqual(["telegram"]);
1241+
expect(plan.healthChecks).toEqual([]);
12161242
expect(plan.channels[0]?.hooks.map((hook) => hook.id)).toEqual([
12171243
"telegram-token-paste",
12181244
"telegram-allowlist-aliases",

‎src/lib/messaging/compiler/manifest-compiler.ts‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,15 @@ export class ManifestCompiler {
6464
planCredentialBindings(manifest, context, inputRegistry.get(manifest.id) ?? []),
6565
);
6666
const networkPolicy = planNetworkPolicy(manifests, context);
67+
const channelRegistry = new Map(
68+
channels.map((channel) => [channel.channelId, channel] as const),
69+
);
70+
const activeManifests = manifests.filter(
71+
(manifest) => channelRegistry.get(manifest.id)?.active === true,
72+
);
6773
const agentRender = (
6874
await Promise.all(
69-
manifests.map((manifest) =>
75+
activeManifests.map((manifest) =>
7076
planAgentRender(
7177
manifest,
7278
context,
@@ -77,12 +83,9 @@ export class ManifestCompiler {
7783
),
7884
)
7985
).flat();
80-
const channelRegistry = new Map(
81-
channels.map((channel) => [channel.channelId, channel] as const),
82-
);
8386
const buildSteps = (
8487
await Promise.all(
85-
manifests.map((manifest) =>
88+
activeManifests.map((manifest) =>
8689
planBuildSteps(
8790
manifest,
8891
context.agent,
@@ -95,7 +98,7 @@ export class ManifestCompiler {
9598
).flat();
9699
const runtimeSetup = planRuntimeSetup(manifests, context.agent, channels);
97100
const stateUpdates = manifests.flatMap((manifest) => planStateUpdates(manifest));
98-
const healthChecks = manifests.flatMap((manifest) => planHealthChecks(manifest));
101+
const healthChecks = activeManifests.flatMap((manifest) => planHealthChecks(manifest));
99102

100103
return {
101104
schemaVersion: 1,

‎src/lib/messaging/compiler/workflow-planner.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -672,6 +672,7 @@ describe("MessagingWorkflowPlanner", () => {
672672
{
673673
MSTEAMS_APP_ID: "test-teams-app-id",
674674
MSTEAMS_TENANT_ID: "test-teams-tenant-id",
675+
TEAMS_ALLOWED_USERS: "00000000-0000-0000-0000-000000000001",
675676
MSTEAMS_PORT: "3977",
676677
},
677678
async () => {

‎src/lib/onboard/messaging-host-forward.test.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,15 @@ describe("ensureMessagingHostForwardIfConfigured", () => {
9393
});
9494
});
9595

96+
it("fails closed when persisted messaging plans are malformed", () => {
97+
expect(
98+
resolveMessagingHostForward({
99+
...makePlan(),
100+
channels: [null],
101+
} as unknown as SandboxMessagingPlan),
102+
).toBeNull();
103+
});
104+
96105
it("starts the active messaging host forward", () => {
97106
const ensureForward = vi.fn(() => true);
98107
const note = vi.fn();

0 commit comments

Comments
 (0)