diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 458bb67e0a..261aece1d1 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -67,6 +67,14 @@ jobs: bump-dev-version: needs: publish if: ${{ inputs.dry-run != true }} + # A reusable-workflow CALL cannot grant the callee more than the calling job holds, + # and GitHub refuses the whole run at startup when the called workflow's own job + # declares permissions the caller did not pass down ("startup_failure", runs + # 33615174183 / 33615177849 — the first dispatches since #3129 wired this call). + # The callee's job declares exactly these two; nothing else in this file gains them. + permissions: + contents: write + pull-requests: write uses: ./.github/workflows/dev-version-bump.yml with: released-version: v${{ inputs.version }} diff --git a/src/service.ts b/src/service.ts index 84b7da3817..8e1e5aae28 100644 --- a/src/service.ts +++ b/src/service.ts @@ -1871,7 +1871,7 @@ export function buildWindowsTaskXml( - InteractiveToken + ${sessionTriggerUserId ? `${taskXmlString(sessionTriggerUserId)}\n ` : ""}InteractiveToken LeastPrivilege @@ -2136,6 +2136,18 @@ function windowsTaskTriggerScopeAcceptable( return expectedValues.some(value => taskXmlDecodedValueEquals(element, "UserId", value)); } +/** The account whose interactive token executes the action must be the current account. */ +function windowsTaskPrincipalScopeAcceptable( + principal: string, + expectedUserId: ExpectedWindowsTaskUserId | undefined, +): boolean { + if (taskXmlHasPrefixedTag(principal, "UserId")) return false; + if (taskXmlElementCount(principal, "UserId") !== 1) return false; + if (expectedUserId === undefined) return false; + const expectedValues = typeof expectedUserId === "string" ? [expectedUserId] : expectedUserId; + return expectedValues.some(value => taskXmlDecodedValueEquals(principal, "UserId", value)); +} + /** Validate the stable OpenCodex action, principal, settings, and logon trigger. */ function windowsTaskRegistrationBaseHealthy( xml: string, @@ -2187,7 +2199,12 @@ export function windowsTaskRegistrationHealthy( ): boolean { const scrubbed = taskXmlWithoutCommentsAndCdata(xml); const triggers = taskXmlSection(scrubbed, "Triggers"); + const principal = taskXmlSection(scrubbed, "Principal"); return windowsTaskRegistrationBaseHealthy(xml, wscript, launcher) + // The trigger scope says whose session event fires the task; Principal/UserId + // independently says whose interactive token executes its action. Both are + // identity boundaries and must match exactly, even when action paths are lossy. + && windowsTaskPrincipalScopeAcceptable(principal, expectedUserId ?? undefined) // Without these the task can only recover at the next logon, so a disconnected session // leaves the proxy down indefinitely. Treating their absence as unhealthy is what lets // an already-registered task from an older install get repaired instead of staying broken. diff --git a/tests/service.test.ts b/tests/service.test.ts index fca80972cc..aa37c764fe 100644 --- a/tests/service.test.ts +++ b/tests/service.test.ts @@ -574,6 +574,21 @@ describe("Windows service task", () => { expect(healthy("C:\\Users\\Admin\\.opencodex\\service-launcher.vbs")).toBe(false); }); + test("rejects a lossy foreign action even when its triggers name the current account", () => { + const victimSid = "S-1-5-21-111-222-333-1001"; + const attackerSid = "S-1-5-21-111-222-333-1002"; + const foreign = buildWindowsTaskXml( + "ignored.cmd", + "C:\\Users\\???\\.opencodex\\service-launcher.vbs", + undefined, + victimSid, + ) + .replace(/.*?<\/Command>/, `${wscript}`) + .replace(`${victimSid}\n `, `${attackerSid}\n `); + + expect(windowsTaskRegistrationHealthy(foreign, wscript, launcher, victimSid)).toBe(false); + }); + test("rejects a path whose ASCII structure differs", () => { expect(healthy("C:\\Users\\???\\.opencodex\\other-launcher.vbs")).toBe(false); expect(healthy("D:\\Users\\???\\.opencodex\\service-launcher.vbs")).toBe(false);