Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/browser-extension.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions .github/workflows/cli-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Minor: build holds contents: write on every run, but only a tag run needs it

Only a cli-v tag run attaches to the release (startsWith(github.ref, 'refs/tags/cli-v')). The job also runs on branch pushes and same-repo pull_request, and it still gets a write token there. actions/checkout keeps that token in .git/config (persist-credentials defaults to true). The risk is low: go build runs no repo code, fork PRs get a read-only token, and it's already a big improvement on write-all. The same applies to checksums.

Suggest moving the two "Attach to release" steps into their own job with if: startsWith(github.ref, 'refs/tags/cli-v'), needs: [build, checksums] and contents: write, so that build and checksums drop back to read. At minimum, set persist-credentials: false on the checkout here.

contents: write
# No observed runs yet; a pure-Go cross-compile is minutes at most.
timeout-minutes: 20
defaults:
Expand Down Expand Up @@ -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:
Expand Down
5 changes: 5 additions & 0 deletions .github/workflows/documentation.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/issue-triage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/openspec-sync.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Minor: the comment describes issue-triage, not openspec-sync

The shared openspec-sync sync job creates and updates issues and creates labels (issues.create, issues.update, issues.createLabel). It doesn't assign anything. The scopes are correct, only the wording is off. Suggest: # What the shared workflow's job declares: it creates and updates the OpenSpec issues and their labels.

permissions:
contents: read
issues: write
with:
app-name: keepiq
secrets:
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/pull-request-lint-check.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ on:
- main
- beta

permissions:
contents: read

jobs:
lint-check:
runs-on: ubuntu-latest
Expand Down
15 changes: 15 additions & 0 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/sync-to-beta.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
16 changes: 15 additions & 1 deletion sdk/js/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
5 changes: 5 additions & 0 deletions sdk/js/test/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
3 changes: 2 additions & 1 deletion tests/e2e/workflows/_vault-users.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down Expand Up @@ -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,
Expand Down
6 changes: 4 additions & 2 deletions tests/extension/accounts.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})

Expand Down Expand Up @@ -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')
})
})
Expand Down
2 changes: 1 addition & 1 deletion tests/extension/unlockAccounts.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
Loading