From 0d278dbe996d202800be4a23c420d40d5391327f Mon Sep 17 00:00:00 2001 From: Robert Zondervan Date: Tue, 6 Oct 2026 15:32:39 +0200 Subject: [PATCH] fix(ci): resolve the open code-scanning alerts on development The 17 alerts GitHub Advanced Security reports on development (and on the development -> beta release PR #834): - actions/missing-workflow-permissions (12): the repository's default token is write-all. Every flagged workflow now declares its permissions: contents: read for the self-contained ones, contents: write only on the cli-release jobs that attach to a release, and for callers of the shared ConductionNL/.github workflows exactly the union the called jobs declare (release, sync-to-beta: contents + pull-requests write; documentation: contents + packages write; issue-triage, openspec-sync: issues write). - js/polynomial-redos (sdk/js): the trailing-slash strip of the caller's address backtracked quadratically on a long run of slashes; replaced by a loop, with a test for an address with trailing slashes. - js/incomplete-url-substring-sanitization (3, extension tests): the assertions compared URL prefixes, which also matched other hosts; they now compare the parsed origin. - js/insecure-randomness (e2e helper): the run-unique user name suffix uses crypto.randomInt instead of Math.random. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/browser-extension.yml | 3 +++ .github/workflows/cli-release.yml | 9 +++++++++ .github/workflows/documentation.yml | 5 +++++ .github/workflows/issue-triage.yml | 4 ++++ .github/workflows/openspec-sync.yml | 4 ++++ .github/workflows/pull-request-lint-check.yaml | 3 +++ .github/workflows/release.yml | 15 +++++++++++++++ .github/workflows/sync-to-beta.yml | 4 ++++ sdk/js/src/client.ts | 16 +++++++++++++++- sdk/js/test/client.test.ts | 5 +++++ tests/e2e/workflows/_vault-users.ts | 3 ++- tests/extension/accounts.spec.js | 6 ++++-- tests/extension/unlockAccounts.spec.js | 2 +- 13 files changed, 74 insertions(+), 5 deletions(-) diff --git a/.github/workflows/browser-extension.yml b/.github/workflows/browser-extension.yml index adf0f8a69..1310697e0 100644 --- a/.github/workflows/browser-extension.yml +++ b/.github/workflows/browser-extension.yml @@ -13,6 +13,9 @@ on: paths: ["browser-extension/**", "src/crypto/**", "src/totp/**", ".github/workflows/browser-extension.yml"] workflow_dispatch: +permissions: + contents: read + jobs: build-and-load: runs-on: ubuntu-latest diff --git a/.github/workflows/cli-release.yml b/.github/workflows/cli-release.yml index 632df2ae2..4a5d39f1c 100644 --- a/.github/workflows/cli-release.yml +++ b/.github/workflows/cli-release.yml @@ -18,6 +18,9 @@ on: paths: ["cli/**", "sdk/go/**", "sdk/testdata/**", ".github/workflows/cli-release.yml"] workflow_dispatch: +permissions: + contents: read + jobs: test: runs-on: ubuntu-latest @@ -43,6 +46,9 @@ jobs: build: needs: test runs-on: ubuntu-latest + # Attaches the binary to the GitHub release on a cli-v tag. + permissions: + contents: write # No observed runs yet; a pure-Go cross-compile is minutes at most. timeout-minutes: 20 defaults: @@ -89,6 +95,9 @@ jobs: needs: build runs-on: ubuntu-latest timeout-minutes: 10 + # Attaches SHA256SUMS to the GitHub release on a cli-v tag. + permissions: + contents: write steps: - uses: actions/download-artifact@v4 with: diff --git a/.github/workflows/documentation.yml b/.github/workflows/documentation.yml index 1aa4c05fc..84fbeb040 100644 --- a/.github/workflows/documentation.yml +++ b/.github/workflows/documentation.yml @@ -15,6 +15,11 @@ on: jobs: deploy: uses: ConductionNL/.github/.github/workflows/documentation.yml@main + # What the shared workflow's jobs declare: the deploy job writes the + # gh-pages branch, the image job publishes the fallback site image. + permissions: + contents: write + packages: write # A reusable workflow receives NO secrets by default. Without this block the # callee's publish step finds CF_API_TOKEN empty, skips itself on its own # `if:` guard, and the run finishes GREEN having changed nothing -- the diff --git a/.github/workflows/issue-triage.yml b/.github/workflows/issue-triage.yml index 05107ddad..aa764a5f8 100644 --- a/.github/workflows/issue-triage.yml +++ b/.github/workflows/issue-triage.yml @@ -13,6 +13,10 @@ on: jobs: triage: uses: ConductionNL/.github/.github/workflows/issue-triage.yml@main + # What the shared workflow's jobs declare: they label and assign issues. + permissions: + contents: read + issues: write with: app-name: keepiq backlog-existing: ${{ github.event_name == 'workflow_dispatch' && inputs.backlog-existing || false }} diff --git a/.github/workflows/openspec-sync.yml b/.github/workflows/openspec-sync.yml index ba89b7a98..5fab43011 100644 --- a/.github/workflows/openspec-sync.yml +++ b/.github/workflows/openspec-sync.yml @@ -9,6 +9,10 @@ on: jobs: sync: uses: ConductionNL/.github/.github/workflows/openspec-sync.yml@main + # What the shared workflow's jobs declare: they label and assign issues. + permissions: + contents: read + issues: write with: app-name: keepiq secrets: diff --git a/.github/workflows/pull-request-lint-check.yaml b/.github/workflows/pull-request-lint-check.yaml index 29a00d0e7..760f7eb11 100644 --- a/.github/workflows/pull-request-lint-check.yaml +++ b/.github/workflows/pull-request-lint-check.yaml @@ -7,6 +7,9 @@ on: - main - beta +permissions: + contents: read + jobs: lint-check: runs-on: ubuntu-latest diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index bf12f102c..6a4ed7ef0 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -37,6 +37,11 @@ jobs: unstable: if: github.ref == 'refs/heads/development' uses: ConductionNL/.github/.github/workflows/release.yml@main + # What the shared release job declares: it commits the version bump, + # tags it and opens the bump pull request. + permissions: + contents: write + pull-requests: write with: release-type: unstable app-name: keepiq @@ -46,6 +51,11 @@ jobs: beta: if: github.ref == 'refs/heads/beta' uses: ConductionNL/.github/.github/workflows/release.yml@main + # What the shared release job declares: it commits the version bump, + # tags it and opens the bump pull request. + permissions: + contents: write + pull-requests: write with: release-type: beta app-name: keepiq @@ -55,6 +65,11 @@ jobs: stable: if: github.ref == 'refs/heads/main' uses: ConductionNL/.github/.github/workflows/release.yml@main + # What the shared release job declares: it commits the version bump, + # tags it and opens the bump pull request. + permissions: + contents: write + pull-requests: write with: release-type: stable app-name: keepiq diff --git a/.github/workflows/sync-to-beta.yml b/.github/workflows/sync-to-beta.yml index 979a37346..3ba96eca3 100644 --- a/.github/workflows/sync-to-beta.yml +++ b/.github/workflows/sync-to-beta.yml @@ -7,4 +7,8 @@ on: jobs: sync: uses: ConductionNL/.github/.github/workflows/sync-to-beta.yml@main + # What the shared workflow's job declares: it opens the sync pull request. + permissions: + contents: write + pull-requests: write secrets: inherit diff --git a/sdk/js/src/client.ts b/sdk/js/src/client.ts index fd6d14b2d..6fd2d29cf 100644 --- a/sdk/js/src/client.ts +++ b/sdk/js/src/client.ts @@ -97,7 +97,7 @@ export class Client { constructor(url: string, private readonly applicationId: string, private readonly privateKeyPem: string, private readonly options: ClientOptions = {}) { const parsed = new URL(url) if (!applicationId) throw new Error('application id is required') - this.base = url.replace(/\/+$/, '') + this.base = stripTrailingSlashes(url) this.origin = parsed.origin this.fetchFn = options.fetch ?? globalThis.fetch.bind(globalThis) this.now = options.now ?? Date.now @@ -313,3 +313,17 @@ export class Client { } } } + +/** + * The address without its trailing slashes. + * + * A loop rather than `/\/+$/`: that pattern backtracks quadratically on a long + * run of slashes that is not at the end, and the address is caller input. + * + * @param url The address + */ +function stripTrailingSlashes(url: string): string { + let end = url.length + while (end > 0 && url[end - 1] === '/') end-- + return url.slice(0, end) +} diff --git a/sdk/js/test/client.test.ts b/sdk/js/test/client.test.ts index 238ea2c90..880e88506 100644 --- a/sdk/js/test/client.test.ts +++ b/sdk/js/test/client.test.ts @@ -37,6 +37,11 @@ describe('Client', () => { expect(stub.exchanges).toBe(1) }) + it('ignores trailing slashes on the address', async () => { + const s = await new Client(url + '///', 'billing', FIXTURE.privateKeyPem).getByName('ci-fixture-db-password') + expect(s.key).toBe(FIXTURE.plaintext.key) + }) + it('reports not modified on an unchanged second read', async () => { const c = client() expect((await c.getById('sec-cli-fixture')).etag).toBeTruthy() diff --git a/tests/e2e/workflows/_vault-users.ts b/tests/e2e/workflows/_vault-users.ts index fdf4b8b3f..b90ae6528 100644 --- a/tests/e2e/workflows/_vault-users.ts +++ b/tests/e2e/workflows/_vault-users.ts @@ -25,6 +25,7 @@ import type { } from '@playwright/test' import { expect, request as playwrightRequest } from '@playwright/test' +import { randomInt } from 'node:crypto' import * as path from 'path' import { APP_BASE } from './_workflow-helpers.ts' @@ -81,7 +82,7 @@ export interface VaultUser { * @return The account details (not created yet). */ export function newVaultUser(prefix: string): VaultUser { - const tag = `${Date.now().toString(36)}${Math.floor(Math.random() * 1296).toString(36)}` + const tag = `${Date.now().toString(36)}${randomInt(1296).toString(36)}` const uid = `${prefix}-${tag}` return { uid, diff --git a/tests/extension/accounts.spec.js b/tests/extension/accounts.spec.js index da50da257..4df8aa190 100644 --- a/tests/extension/accounts.spec.js +++ b/tests/extension/accounts.spec.js @@ -117,7 +117,9 @@ describe('account storage', () => { expect(sixth.error).toContain('up to 5 accounts') expect(await api.loadAccounts()).toHaveLength(5) expect( - server.calls.some((c) => c.url.startsWith('https://cloud6.example')), + server.calls.some( + (c) => new URL(c.url).origin === 'https://cloud6.example', + ), ).toBe(false) }) @@ -296,7 +298,7 @@ describe('matching and filling use the active account only', () => { (c) => c.method === 'POST' && c.url.endsWith('/api/v1/secrets'), ) expect(posts).toHaveLength(1) - expect(posts[0].url.startsWith(HOME)).toBe(true) + expect(new URL(posts[0].url).origin).toBe(HOME) expect(posts[0].body.encryptionSuiteId).toBe('home-suite') }) }) diff --git a/tests/extension/unlockAccounts.spec.js b/tests/extension/unlockAccounts.spec.js index b780e596d..f2d49e932 100644 --- a/tests/extension/unlockAccounts.spec.js +++ b/tests/extension/unlockAccounts.spec.js @@ -160,7 +160,7 @@ describe('log out and lock', () => { c.method === 'DELETE' && c.url.endsWith('/ocs/v2.php/core/apppassword'), ) - expect(revokes.map((c) => c.url.startsWith(TWO))).toEqual([true]) + expect(revokes.map((c) => new URL(c.url).origin)).toEqual([TWO]) }) it('logs out of all accounts', async () => {