diff --git a/.github/allowed_signers b/.github/allowed_signers index 87ce163..e00d9a0 100644 --- a/.github/allowed_signers +++ b/.github/allowed_signers @@ -5,11 +5,11 @@ # v* tag against the copy of this file on main; a tag signed by any other key, or not signed, # stops the release before anything is built. GPG keys go in .github/release-keys.asc instead. # -# One line per key, the same keys as in MAINTAINERS.md. Adding or removing a key is a reviewed -# pull request. Format (see "ALLOWED SIGNERS" in `man ssh-keygen`): +# One line per key, the same keys as in MAINTAINERS.md. A key is added or removed by pull +# request; a second maintainer's review isn't enforced yet (docs/security/assurance-case.md, +# residual risks). Format (see "ALLOWED SIGNERS" in `man ssh-keygen`): # # you@example.com namespaces="git" ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAA... # - # Ayla Croft (@HackTuah), release manager: SHA256:HqIaFq/thUm844NjYC3a+4Br40cqD2yJHEq1L2iiw4E dev@scriptkittyos.com namespaces="git" ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIAiWNJRQYwtCxb7uEMB5i1XdR6k9Xd+hF6+kMQa4nQ3j dev@scriptkittyos.com diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e95da66..cf70a77 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,6 +20,10 @@ jobs: # .env, database, key or oversized file in the tree or a pull request's commits (GitHub's # push rulesets work only on private repositories), and every commit a pull request adds # is signed off (the Developer Certificate of Origin, CONTRIBUTING.md). + # + # On a pull request, gitleaks' settings and the check scripts come from the base commit, + # so a pull request can't switch off its own checks (security review C-01): its edits to + # them take effect once it is merged. On main they come from the tree. secrets: runs-on: ubuntu-24.04 env: @@ -36,21 +40,50 @@ jobs: "https://github.com/gitleaks/gitleaks/releases/download/v${GITLEAKS_VERSION}/gitleaks_${GITLEAKS_VERSION}_linux_x64.tar.gz" echo "${GITLEAKS_SHA256} $RUNNER_TEMP/gitleaks.tar.gz" | sha256sum -c - tar -xzf "$RUNNER_TEMP/gitleaks.tar.gz" -C "$RUNNER_TEMP" gitleaks + # Security review C-01's regression test, with this gitleaks: a pull request that adds a + # token and allows it in its own .gitleaks.toml and .gitleaksignore still fails. + - name: A pull request can't switch off its own scan (test) + env: + GITLEAKS: ${{ runner.temp }}/gitleaks + run: node --test test/unit/pr-checks-base.test.mjs + - name: The checks' settings and scripts, from the base commit on a pull request + env: + BASE: ${{ github.event.pull_request.base.sha }} + run: | + ref="${BASE:-HEAD}" + git cat-file -e "$ref^{commit}" + # The base's own copy of the helper. Only the pull request that adds it has none. + if git cat-file -e "$ref:scripts/base-file.sh" 2>/dev/null; then + git show "$ref:scripts/base-file.sh" > "$RUNNER_TEMP/base-file.sh" + else + echo "::warning title=No base-file.sh on the base::Using this tree's copy; only the pull request that adds it should see this." + cp scripts/base-file.sh "$RUNNER_TEMP/base-file.sh" + fi + bash "$RUNNER_TEMP/base-file.sh" "$ref" "$RUNNER_TEMP/trusted" \ + .gitleaks.toml .gitleaksignore scripts/check-forbidden-files.mjs scripts/check-dco.mjs + # gitleaks also reads the .gitleaksignore of the folder it scans, whatever + # --gitleaks-ignore-path says, so the tree's copies go. + rm -f .gitleaks.toml .gitleaksignore - name: Scan the full history # HEAD's full history (all of main plus this pull request's commits). Not --all: the # checkout fetches every branch, and another branch's finding would fail this one. - run: '"$RUNNER_TEMP/gitleaks" git --log-opts="HEAD" --redact --config .gitleaks.toml --exit-code 1 .' + # Inline "gitleaks:allow" comments don't count: a pull request could add one. + run: >- + "$RUNNER_TEMP/gitleaks" git --log-opts="HEAD" --redact --exit-code 1 + --config "$RUNNER_TEMP/trusted/.gitleaks.toml" --gitleaks-ignore-path "$RUNNER_TEMP/trusted/.gitleaksignore" --ignore-gitleaks-allow + . - name: No secret, database or oversized files env: BASE: ${{ github.event.pull_request.base.sha }} HEAD: ${{ github.event.pull_request.head.sha }} - run: node scripts/check-forbidden-files.mjs $BASE $HEAD + # Without a pull request (a push to main), the tree only. + run: node "$RUNNER_TEMP/trusted/scripts/check-forbidden-files.mjs" ${BASE:+"$BASE" "$HEAD"} - name: Every commit is signed off (DCO) if: github.event_name == 'pull_request' env: BASE: ${{ github.event.pull_request.base.sha }} HEAD: ${{ github.event.pull_request.head.sha }} - run: node scripts/check-dco.mjs "$BASE" "$HEAD" + run: node "$RUNNER_TEMP/trusted/scripts/check-dco.mjs" "$BASE" "$HEAD" server: name: server (${{ matrix.elixir }}) diff --git a/.github/workflows/dependency-scan.yml b/.github/workflows/dependency-scan.yml index bc0c093..21e52e9 100644 --- a/.github/workflows/dependency-scan.yml +++ b/.github/workflows/dependency-scan.yml @@ -42,7 +42,26 @@ jobs: steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: + fetch-depth: 0 # the base commit, for its osv-scanner.toml persist-credentials: false + # On a pull request, the accepted findings come from the base commit's osv-scanner.toml, + # so a pull request can't accept its own vulnerable dependency (security review C-01); + # its edits to the file take effect once it is merged. Elsewhere (main, the weekly run, + # a release) they come from the tree. + - name: The scan's settings, from the base commit on a pull request + env: + BASE: ${{ github.event.pull_request.base.sha }} + run: | + ref="${BASE:-HEAD}" + git cat-file -e "$ref^{commit}" + # The base's own copy of the helper. Only the pull request that adds it has none. + if git cat-file -e "$ref:scripts/base-file.sh" 2>/dev/null; then + git show "$ref:scripts/base-file.sh" > "$RUNNER_TEMP/base-file.sh" + else + echo "::warning title=No base-file.sh on the base::Using this tree's copy; only the pull request that adds it should see this." + cp scripts/base-file.sh "$RUNNER_TEMP/base-file.sh" + fi + bash "$RUNNER_TEMP/base-file.sh" "$ref" "$RUNNER_TEMP/trusted" osv-scanner.toml - name: Install OSV-Scanner run: | curl -sSfL -o "$RUNNER_TEMP/osv-scanner" \ @@ -52,7 +71,7 @@ jobs: - name: Scan the lockfiles run: | set +e - "$RUNNER_TEMP/osv-scanner" scan source --config osv-scanner.toml \ + "$RUNNER_TEMP/osv-scanner" scan source --config "$RUNNER_TEMP/trusted/osv-scanner.toml" \ --lockfile package-lock.json \ --lockfile site/package-lock.json \ --lockfile server/mix.lock \ diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 96f0ef6..9119d71 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -56,6 +56,7 @@ jobs: outputs: version: ${{ steps.verify.outputs.version }} prerelease: ${{ steps.verify.outputs.prerelease }} + commit: ${{ steps.verify.outputs.commit }} steps: # The checking script and the allowed signers come from main, never from the tag being # checked, so a tag can't vouch for itself. @@ -93,12 +94,30 @@ jobs: contents: read outputs: version: ${{ steps.version.outputs.version }} + env: + # Fixed seeds for the property tests, so a release's tests are the same on every run and + # on a rerun (security review C-08). CI keeps a new seed on every pull request and push + # to main, so it keeps exploring; what fails there is fixed before it reaches a release. + FC_SEED: "20261006" + MIX_TEST_SEED: "20261006" steps: # The tag on a tag push; the chosen branch on a manual run. No caches in this workflow, # so nothing a pull request wrote can reach a release. - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false + # What is built is the commit verify-tag checked, nothing else (security review C-02). + - name: The checked-out commit is the verified one + if: github.event_name == 'push' + env: + VERIFIED: ${{ needs.verify-tag.outputs.commit }} + run: | + head="$(git rev-parse HEAD)" + if [ -z "$VERIFIED" ] || [ "$head" != "$VERIFIED" ]; then + echo "::error title=Not the verified commit::verify-tag checked ${VERIFIED:-no commit}, but the checkout is $head." + exit 1 + fi + echo "Building $head, the commit verify-tag checked." - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: node-version: 22 @@ -118,7 +137,8 @@ jobs: - run: npm ci # The same checks as CI, on the tree being released: the extension's and the server's # tests with their coverage gates. The property tests run in both, so the dynamic - # analysis runs before every release. + # analysis runs before every release, with the fixed seeds above (StreamData takes + # ExUnit's --seed). - run: npm run lint - run: npm run coverage - name: Server tests @@ -127,7 +147,7 @@ jobs: MIX_ENV: test run: | mix deps.get - mix test --cover + mix test --cover --seed "$MIX_TEST_SEED" - name: Build the zips twice and compare env: VERSION: ${{ steps.version.outputs.version }} @@ -247,7 +267,11 @@ jobs: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false - - run: node scripts/check-security-gate.mjs + # The lead's exact "Gate: closed" line (slice 54 §6); a review opened for this version + # must be the closed one. + - env: + VERSION: ${{ needs.verify-tag.outputs.version }} + run: node scripts/check-security-gate.mjs --version "$VERSION" publish-chrome: needs: [verify-tag, store-gate] @@ -354,20 +378,16 @@ jobs: with: name: release-files path: dist - - if: steps.creds.outputs.ready == 'true' - name: Check the zip and unpack it - env: - VERSION: ${{ needs.verify-tag.outputs.version }} - run: | - (cd dist && sha256sum -c SHA256SUMS) - unzip -q "dist/kotiko-firefox-$VERSION.zip" -d "$RUNNER_TEMP/firefox" # Listed on addons.mozilla.org: submit and don't wait for review. AMO signs the version - # after review and sends it to users itself. + # after review and sends it to users itself. The file submitted is the release's own + # zip, checked against SHA256SUMS first, not a zip web-ext builds again (security review + # C-06); scripts/amo-submit.mjs explains. - if: steps.creds.outputs.ready == 'true' - name: Submit to Firefox Add-ons + name: Submit the release zip to Firefox Add-ons env: WEB_EXT_API_KEY: ${{ secrets.AMO_JWT_ISSUER }} WEB_EXT_API_SECRET: ${{ secrets.AMO_JWT_SECRET }} - run: >- - npx --no-install web-ext sign --no-config-discovery --channel listed - --source-dir "$RUNNER_TEMP/firefox" --artifacts-dir "$RUNNER_TEMP/signed" --approval-timeout 0 + VERSION: ${{ needs.verify-tag.outputs.version }} + run: | + (cd dist && sha256sum -c SHA256SUMS) + node scripts/amo-submit.mjs --zip "dist/kotiko-firefox-$VERSION.zip" --sums dist/SHA256SUMS diff --git a/SECURITY.md b/SECURITY.md index 0d1005b..8cbf692 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -2,7 +2,7 @@ ## Supported versions -Kotiko is before 1.0. Only the latest minor release gets security fixes. +Only the latest release gets security fixes: from 1.0.0 on (the first store release, still pending), the latest 1.x. Release candidates (`vX.Y.Z-rc.N` pre-releases, for review) and versions older than 1.0.0 get none. ## Reporting a vulnerability diff --git a/docs/reproducible-builds.md b/docs/reproducible-builds.md index 8ebec59..2dd4f2a 100644 --- a/docs/reproducible-builds.md +++ b/docs/reproducible-builds.md @@ -1,8 +1,11 @@ # Reproducible builds Anyone can rebuild a release's extension zips from its tag and get the same bytes as the -files on the GitHub release and in the stores. The release workflow checks this on every -release by building twice and comparing; this page is how you check it yourself. +zips on the GitHub release, which are the files the release workflow uploads to the Chrome +Web Store and Firefox Add-ons. (The stores then sign what they distribute, which adds their +signature to the same files: [verify.md](verify.md#3-the-store-versions).) The release +workflow checks this on every release by building twice and comparing; this page is how you +check it yourself. ## Rebuild a release diff --git a/docs/security/assurance-case.md b/docs/security/assurance-case.md index cf1e994..6b19d16 100644 --- a/docs/security/assurance-case.md +++ b/docs/security/assurance-case.md @@ -65,7 +65,7 @@ its argument and evidence. | A Telegram user who isn't allowed | The bot's chat | Sender allowlist | | Another user on the server machine | Files in the data folder, depending on permissions | Every file the server keeps there is 0600 (the database is created private before SQLite opens it, so `-wal` and `-shm` are too) and a folder it makes 0700; each start re-checks them; `run.sh` and the systemd unit use umask 077 | | A compromised dependency or GitHub Action | The build, CI, and later the release | Lockfiles, actions pinned by commit SHA, Dependabot, OSV-Scanner over every lockfile in CI and before each release, `mix hex.audit` and `mix deps.audit` in CI, no runtime npm code in the extension | -| A malicious pull request | The code base | Review by a maintainer; CI with read-only token; branch protection on going public ([PUBLIC_CHECKLIST.md](../PUBLIC_CHECKLIST.md)) | +| A malicious pull request | The code base | Required CI checks before merging (the `main` ruleset), with a read-only token; on a pull request the secret and dependency scans take their allowlists and check scripts from the base commit, so it can't switch them off ([`base-file.sh`](../../scripts/base-file.sh)). Review by a maintainer who isn't the author is the aim but not enforced (section 7) | ## 3. Trust boundaries @@ -100,7 +100,7 @@ its argument and evidence. | Server or extension to model provider | The learner's typed text, the key | HTTPS to an address the owner set; answers parsed as JSON and checked by the word spec; deadline and attempt cap per lookup | [`llm.ex`](../../server/lib/kotiko/llm.ex) `interpret/3`; [`spec/models.json`](../../spec/models.json) budgets; [`lib/llm/client.js`](../../extension/lib/llm/client.js) | | Server to Telegram | Updates in, messages out | HTTPS; sender allowlist | [`bot.ex`](../../server/lib/kotiko/bot.ex); [`telegram.ex`](../../server/lib/kotiko/telegram.ex) | | Server to file system | The database, token, backups | `kotiko.db` and its `-wal`/`-shm`, `api-token`, `models-cache.json` 0600; `backups/` 0700 with 0600 files; a data folder the server makes 0700, one that holds only Kotiko's files made 0700 at start (otherwise a warning); umask 077 in `run.sh` and the unit (`UMask=0077`) | [`private.ex`](../../server/lib/kotiko/private.ex); [`data_dir.ex`](../../server/lib/kotiko/data_dir.ex) `make_private/1`; [`migrations.ex`](../../server/lib/kotiko/migrations.ex) `backup!/2`; [`data_dir_test.exs`](../../server/test/kotiko/data_dir_test.exs); [`boot_test.exs`](../../server/test/kotiko/boot_test.exs) | -| CI to stores | Release packages | The store jobs run in the `release` environment: a maintainer approves each run, and only `v*` tags (signed, verified against `.github/allowed_signers`) may deploy. The maintainer creates the environment before the first release | [slice 30](../../slices/30-release-pipeline/SPEC.md) | +| CI to stores | Release packages | The store jobs run in the `release` environment (created 2026-10-06): its required reviewer, the maintainer, approves each run, and only `v*` tags may deploy. A tag is built only if it is signed by a key in `.github/allowed_signers` on `main`, under its own name, and only the release manager can create one. The stores get the release's own zips, checked against `SHA256SUMS` | [slice 30](../../slices/30-release-pipeline/SPEC.md); [`release.yml`](../../.github/workflows/release.yml); [`verify-tag.sh`](../../scripts/verify-tag.sh) | ## 4. Secure design principles @@ -139,7 +139,7 @@ a small JSON API. | CWE-798 Hard-coded credentials | No credentials in the code. CI's required `secrets` job runs gitleaks over the full history on every pull request and every push to `main`, and fails a pull request that adds a `.env`, key or token file; GitHub secret scanning and push protection are on | In place | [`ci.yml`](../../.github/workflows/ci.yml) job `secrets`; [`.gitleaks.toml`](../../.gitleaks.toml); [`check-forbidden-files.mjs`](../../scripts/check-forbidden-files.mjs) | | CWE-532 Secrets in logs | The redaction filter; words only with `LOG_LOOKUPS=true` at debug | In place | [`log/redact.ex`](../../server/lib/kotiko/log/redact.ex); [`llm_test.exs`](../../server/test/kotiko/llm_test.exs) | | DNS rebinding | Host allowlist before routing | In place | [`host_check.ex`](../../server/lib/kotiko/plug/host_check.ex) | -| Supply chain | Lockfiles (`mix.lock`, `package-lock.json`); every GitHub Action pinned by commit SHA; Dependabot for mix, both npm lockfiles and Actions; OSV-Scanner over all three lockfiles on every pull request, weekly and before each release build; `mix hex.audit` and `mix deps.audit` in CI; no runtime npm code in the extension; review on every pull request ([policy](dependency-and-static-analysis-policy.md)). Not yet: release attestations (slice 30) | In place, partly | [`dependency-scan.yml`](../../.github/workflows/dependency-scan.yml); [`ci.yml`](../../.github/workflows/ci.yml); [`dependabot.yml`](../../.github/dependabot.yml); [`package.json`](../../package.json) | +| Supply chain | Lockfiles (`mix.lock`, `package-lock.json`); every GitHub Action pinned by commit SHA; Dependabot for mix, both npm lockfiles and Actions; OSV-Scanner over all three lockfiles on every pull request, weekly and before each release build; `mix hex.audit` and `mix deps.audit` in CI; no runtime npm code in the extension; required CI checks on every pull request, though not a second person's review ([policy](dependency-and-static-analysis-policy.md)). Not yet: release attestations (slice 30) | In place, partly | [`dependency-scan.yml`](../../.github/workflows/dependency-scan.yml); [`ci.yml`](../../.github/workflows/ci.yml); [`dependabot.yml`](../../.github/dependabot.yml); [`package.json`](../../package.json) | ## 6. Inputs and how each is validated @@ -195,6 +195,14 @@ These match "What you can't expect" in the [requirements](requirements.md). - **No rate limit** for requests that carry the token. - **Response hardening headers** (`X-Content-Type-Options` and similar) are not sent yet; only `/health` sends `cache-control: no-store`. Planned (slice 01 section 7). +- **No enforced second review.** The `main` ruleset requires the CI checks but no + approving review, so one maintainer account (or a stolen token with write access) can + merge its own pull request, including changes to `.github/allowed_signers` and the + workflows. Accepted while one maintainer does nearly all the work + ([CODE_REVIEW.md](../CODE_REVIEW.md), step 2). The release tag and the store upload still + need the release manager: only they can create a `v*` tag, and the `release` + environment's reviewer approves each upload. A pull request can also change a workflow + itself, and that change runs on the pull request; only review catches it. - **No releases yet**, so there are no signatures or attestations to verify; users run from source. The release pipeline that signs, attests and builds reproducibly is in place (slice 30); the first release is v1.0.0. diff --git a/docs/security/requirements.md b/docs/security/requirements.md index 8f81cf5..f09afc2 100644 --- a/docs/security/requirements.md +++ b/docs/security/requirements.md @@ -105,7 +105,9 @@ Kotiko has two parts: the browser extension, and an optional server you run your These are goals in the plan. They move to "What you can expect" when the slice ships. - Signed, attested release packages and store-reviewed builds: slice 30 (release - pipeline). There are no releases yet; today you run Kotiko from source. + pipeline). The pipeline is in place, and release candidates (`v1.0.0-rc.N`) are tagged + for the pre-release security review (slice 54); the first store release, v1.0.0, is + pending. Until it is out, you run Kotiko from source. - Hardening headers on every server response: slice 01 (addition from slice 53). (The extension's pages already have an explicit content security policy, `manifest.json`.) - A permission review of the extension against the store's rules: slice 28. diff --git a/docs/stores.md b/docs/stores.md index 6752697..f74f4eb 100644 --- a/docs/stores.md +++ b/docs/stores.md @@ -57,8 +57,10 @@ release); from then on release-please picks the version from the commits: `fix` 5. **Approve the store uploads.** The two store jobs wait for approval in the `release` environment (Actions, the run, "Review deployments"). Before asking, the workflow checks that slice 54's security review is closed; until it is, the store jobs fail and nobody is - asked to approve. After approval, Chrome gets the zip and submits it for review; Firefox - gets the unpacked zip through `web-ext sign --channel listed` and doesn't wait for review. + asked to approve. After approval, each store gets the zip from the GitHub release itself, + checked against `SHA256SUMS` first: Chrome submits it for review; Firefox gets it as a new + listed version through AMO's API (`scripts/amo-submit.mjs`, which uses web-ext's AMO + client but not `web-ext sign`, because that builds a new zip) and doesn't wait for review. Both jobs succeed on "submitted"; review takes hours to days. 6. **Afterwards.** A daily job (`.github/workflows/store-status.yml`) writes the Chrome Web Store's review status into the release notes until the version is live. Once AMO @@ -76,6 +78,20 @@ git switch --detach origin/release-please--branches--main scripts/tag-release.sh --rc 1 --ssh-key ~/.ssh/kotiko_release.pub ``` +**When a tag's run fails** (a test that failed by chance, a runner problem), run the failed +jobs again for the same tag; there is no need for a new tag or candidate number: + +```sh +gh run list --workflow release.yml --branch v1.0.0-rc.1 --limit 1 # the run's id +gh run rerun --failed +``` + +`--failed` reruns the failed jobs and the jobs after them, on the same commit; verify-tag's +result and the files of jobs that passed are kept. The release build's property tests use +fixed seeds (`FC_SEED`, and `mix test --seed`), so a rerun runs the same inputs; CI keeps a +new seed on every pull request. A failure that comes back on the rerun is a real one: fix it +on `main` and tag the next candidate. + ### Trying the pipeline without releasing Actions, "Release", "Run workflow", pick a branch. A manual run is always a dry run: it runs @@ -95,24 +111,38 @@ All of these are the maintainer's to do; nothing in the repository can do them. - **Settings, Actions, General**: "Allow GitHub Actions to create and approve pull requests" (release-please opens the release PR with the workflow's token). -- **Settings, Environments, `release`**: - - Required reviewers: the maintainers (MAINTAINERS.md), at least one. Leave "Prevent - self-review" off, so the release manager can approve their own release. +- **Settings, Environments, `release`** (created 2026-10-06; until it existed, GitHub would + have created it on first use with no protection): + - Required reviewers: the maintainers (MAINTAINERS.md), at least one; today the + maintainer. Leave "Prevent self-review" off, so the release manager can approve their + own release. Every store upload waits for this approval. - Deployment branches and tags: "Selected branches and tags", add the **tag** rule `v*` and no branches. Store jobs run on the release tag, never on a branch, a pull request or a fork. - Secrets: the six store secrets below. Nothing else in the repository can read them. - **Settings, Environments, `store-status`**: no reviewers; deployment branches: `main` only. Secrets: the read-only Chrome Web Store credentials below. -- **Settings, Rules, Rulesets**: "tags are permanent" (since 2026-10-06, as in the - maintainer's other projects) covers every tag: no updates, deletions or force pushes, and - no bypass. Only collaborators with write access (the maintainers) can push a tag at all, - and the release workflow builds only a tag signed by a key in `.github/allowed_signers`, - so a tag can't be moved after a release and an unsigned one releases nothing. Two more rulesets: `main` takes changes only through pull requests whose CI checks - pass (no force pushes or deletion, no bypass). GitHub allows push rulesets only on - private repositories, so the old push ruleset is disabled; CI's required `secrets` job - rejects `.env`, database, private-key and token files and files over 10 MB instead - (`scripts/check-forbidden-files.mjs`), along with unsigned commits (the DCO). +- **Settings, Rules, Rulesets**: + - "tags are permanent" (since 2026-10-06, as in the maintainer's other projects) covers + every tag: no updates, deletions or force pushes. The one exception is the release + manager (the repository admin), who can delete a wrong release tag (below). + - Release tags (`v*`): only the release manager (the repository admin) can create or + delete them; other maintainers can't, so nobody else can take a version's name first. + The release workflow builds only a tag signed by a key in `.github/allowed_signers`, + under the name it was signed with, so a tag can't be moved after a release and an + unsigned or renamed one releases nothing. + - `main` takes changes only through pull requests whose required CI checks pass (no force + pushes or deletion, no bypass). It doesn't require an approving review: a maintainer + can merge their own pull request once the checks pass. That is an accepted risk while + one maintainer does nearly all the work ([CODE_REVIEW.md](CODE_REVIEW.md), step 2); + review by a second maintainer is the aim, not yet enforced. + - GitHub allows push rulesets only on private repositories, so the old push ruleset is + disabled; CI's required `secrets` job rejects `.env`, database, private-key and token + files and files over 10 MB instead (`scripts/check-forbidden-files.mjs`), along with + unsigned commits (the DCO). On a pull request, that job and the dependency scan take + their settings (`.gitleaks.toml`, `.gitleaksignore`, `osv-scanner.toml`) and these + check scripts from the base commit (`scripts/base-file.sh`), so a pull request can't + switch off its own checks; its edits to them apply once it is merged. - **Settings, Secrets and variables, Actions, Variables** (optional): `CHROME_STORE_URL` and `FIREFOX_STORE_URL` (the listing pages, used in release notes once the items exist), `CWS_DEPLOY_PERCENTAGE` (a staged rollout, 1 to 100; leave unset to @@ -221,12 +251,22 @@ prompt (07, 09): ### The security gate in the workflow -The `store-gate` job passes only when the tagged tree has a -`docs/security/review-vX.Y.Z[-rc.N].md` with a line starting `Gate: closed` (for example -`Gate: closed 2026-11-02 by , fixes confirmed on v1.0.0-rc.2`), which slice 54's lead -writes when the review ends. That blocks every store upload until the first review is done. -Later reviews (for releases that touch the areas above) are enforced by the checklist box, -not by the workflow, because no script can tell which releases need one. +The `store-gate` job (`scripts/check-security-gate.mjs`) passes only when the tagged tree has +a `docs/security/review-vX.Y.Z-rc.N.md` that slice 54's lead closed when the review ended, +with exactly one line of this form, on its own, above the report's `## Appendix` heading +(under which the reviewers' reports are appended): + +```text +Gate: closed 2026-11-02 by , fixes confirmed on v1.0.0-rc.2 +``` + +The candidate it names is one of the reviewed version's. A second `Gate:` line (for example +`Gate: open`) above the appendix keeps the gate open, and lines under the appendix count for +nothing. That blocks every store upload until the first review is done. When a review is +opened for the version being released (a `review-vX.Y.Z-rc.N.md` for that version), that +review must be the closed one. Whether a later release needs a review at all (it touches +the areas above) is enforced by the checklist box, not by the workflow, because no script +can tell which releases need one. ## Rollback @@ -238,11 +278,15 @@ until the fix is approved. On the Chrome Web Store, a staged rollout ## When something goes wrong -- **"isn't signed" or "not in the allowed signers file"**: the tag was made without `-s`, or - with a key not in `.github/allowed_signers` (or `.github/release-keys.asc`) on `main`. - Delete the tag (`git push origin :refs/tags/vX.Y.Z` and `git tag -d vX.Y.Z`; the ruleset - lets maintainers do this), fix the key, run `scripts/tag-release.sh` again. Nothing was - built or published. +- **"isn't signed", "not in the allowed signers file" or "signed as … not …"**: the tag was + made without `-s`, with a key not in `.github/allowed_signers` (or + `.github/release-keys.asc`) on `main`, or it holds another tag's signed object. Nothing + was built or published. Only the release manager (the repository admin) can delete a + wrong release tag; other maintainers can't. The release manager deletes it + (`git push origin :refs/tags/vX.Y.Z` and `git tag -d vX.Y.Z`), fixes the key, and runs + `scripts/tag-release.sh` again. +- **A test failed in the release build**: rerun the failed jobs for the same tag + (`gh run rerun --failed`, [above](#release-candidates)). - **release-please doesn't open the next PR**: the last release PR still has the label `autorelease: pending`. The workflow moves it to `autorelease: tagged`; if it warned that it couldn't find the PR, change the label by hand. diff --git a/docs/verify.md b/docs/verify.md index 5d13e90..212cff6 100644 --- a/docs/verify.md +++ b/docs/verify.md @@ -1,7 +1,8 @@ # Verifying a Kotiko release -Each release on GitHub has the two extension zips (the exact packages sent to the Chrome Web -Store and Firefox Add-ons), the server's SBOM (`kotiko-server-X.Y.Z.cdx.json`) and +Each release on GitHub has the two extension zips (the exact files the release workflow +uploads to the Chrome Web Store and Firefox Add-ons; [section 3](#3-the-store-versions) says +what the stores add), the server's SBOM (`kotiko-server-X.Y.Z.cdx.json`) and `SHA256SUMS`. These steps check that what you downloaded was built by this repository's release workflow from a tag a maintainer signed. Replace `1.0.0` with the version you have. @@ -23,6 +24,18 @@ The output starts with `Good "git" signature for `. For a GPG-signed the release keys first: `gpg --import .github/release-keys.asc`, then `git verify-tag v1.0.0` shows "Good signature". +Then check that the signed tag carries this tag's name. The signature covers the tag object, +not the name you fetched it under, so a release candidate's signed object could be pushed +again as `v1.0.0`: + +```sh +git cat-file tag v1.0.0 | sed -n '/^$/q; s/^tag //p' +``` + +This prints `v1.0.0`, the name you asked for. Anything else (for example `v1.0.0-rc.2`) +means the tag isn't the release the maintainer signed. The release workflow refuses such a +tag (`scripts/verify-tag.sh`). + Cross-check the key: its fingerprint should match MAINTAINERS.md and the signing keys on the maintainer's GitHub profile (`https://github.com/.keys` for SSH, `https://github.com/.gpg` for GPG). A key that appears only in the repository proves @@ -53,10 +66,13 @@ See [reproducible-builds.md](reproducible-builds.md). ## 3. The store versions -Install Kotiko only from the store links in the [README](../README.md). The stores sign -what they distribute: Firefox Add-ons signs every add-on after review, and the Chrome Web -Store signs its packages. A zip from somewhere else, loaded unpacked, gets none of these -checks and no automatic updates. +Install Kotiko only from the store links in the [README](../README.md). The release workflow +uploads the two zips from the GitHub release to the stores unchanged, after checking each +against `SHA256SUMS` (Firefox Add-ons: `scripts/amo-submit.mjs`). The stores then sign what +they distribute, so the package you install isn't byte-identical to the zip: Firefox Add-ons +adds Mozilla's signature files (`META-INF/`) after review, and the Chrome Web Store +repackages the zip as a signed CRX. The extension's own files inside are the same. A zip from +somewhere else, loaded unpacked, gets none of these checks and no automatic updates. ## What the zips contain diff --git a/extension/manifest.json b/extension/manifest.json index 1daf506..eb73c63 100644 --- a/extension/manifest.json +++ b/extension/manifest.json @@ -48,7 +48,7 @@ "gecko": { "id": "kotiko@scriptkittyos.com", "strict_min_version": "140.0", - "data_collection_permissions": { "required": ["none"] } + "data_collection_permissions": { "required": ["authenticationInfo"] } }, "gecko_android": { "strict_min_version": "142.0" } } diff --git a/scripts/amo-submit.mjs b/scripts/amo-submit.mjs new file mode 100644 index 0000000..46f226a --- /dev/null +++ b/scripts/amo-submit.mjs @@ -0,0 +1,89 @@ +#!/usr/bin/env node +// SPDX-FileCopyrightText: 2026 ScriptKittyOS and the Kotiko contributors +// SPDX-License-Identifier: Apache-2.0 + +// Submits the release's own Firefox zip to addons.mozilla.org as a new listed version +// (slice 30; security review C-06). Run by .github/workflows/release.yml: +// +// node scripts/amo-submit.mjs --zip dist/kotiko-firefox-1.0.0.zip --sums dist/SHA256SUMS +// +// `web-ext sign` can't do this: it always builds a new zip from a folder, with new +// timestamps, so the package AMO reviewed could not be matched to SHA256SUMS or the release's +// attestation. This script first checks the zip's SHA-256 against its SHA256SUMS line, then +// hands exactly that file to web-ext's own AMO client (web-ext/util/submit-addon, the AMO v5 +// API: upload, validation, new version), which sends the file's bytes unchanged. It doesn't +// wait for review. AMO signs the version after review and distributes the signed copy, which +// adds Mozilla's signature files (META-INF/) to the same files. +// +// Credentials come from the environment: WEB_EXT_API_KEY (the JWT issuer) and +// WEB_EXT_API_SECRET. --id defaults to the gecko id in extension/manifest.json. + +import { createHash } from "node:crypto"; +import { mkdtempSync, readFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { basename, dirname, join, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; + +const ROOT = resolve(dirname(fileURLToPath(import.meta.url)), ".."); +export const AMO_BASE_URL = "https://addons.mozilla.org/api/v5/"; + +export const sha256 = (file) => createHash("sha256").update(readFileSync(file)).digest("hex"); + +/** The SHA-256 that a sha256sum-style file gives for name, or null. */ +export function listedHash(sumsText, name) { + for (const line of sumsText.split(/\r?\n/)) { + const m = line.match(/^([0-9a-f]{64}) [ *](.+)$/); + if (m && m[2] === name) return m[1]; + } + return null; +} + +/** Checks the zip against SHA256SUMS and submits that exact file; returns its SHA-256. */ +export async function submit({ zip, sums, id, apiKey, apiSecret, amoBaseUrl = AMO_BASE_URL, signAddon }) { + if (!apiKey || !apiSecret) throw new Error("WEB_EXT_API_KEY and WEB_EXT_API_SECRET must be set"); + const expected = listedHash(readFileSync(sums, "utf8"), basename(zip)); + if (!expected) throw new Error(`${basename(zip)} isn't listed in ${sums}`); + const actual = sha256(zip); + if (actual !== expected) throw new Error(`${basename(zip)} has SHA-256 ${actual}, but ${sums} says ${expected}`); + const send = signAddon ?? (await import("web-ext/util/submit-addon")).signAddon; + // A fresh folder for web-ext's upload cache, so nothing from another run is reused. + const scratch = mkdtempSync(join(tmpdir(), "kotiko-amo-")); + try { + await send({ + apiKey, + apiSecret, + amoBaseUrl, + id, + xpiPath: zip, + channel: "listed", + approvalCheckTimeout: 0, + savedUploadUuidPath: join(scratch, "upload-uuid.json"), + downloadDir: scratch, + userAgentString: "kotiko-release", + }); + } finally { + rmSync(scratch, { recursive: true, force: true }); + } + return actual; +} + +if (process.argv[1] && resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { + const arg = (name) => { + const i = process.argv.indexOf(name); + return i > 0 ? process.argv[i + 1] : undefined; + }; + const zip = arg("--zip"); + const sums = arg("--sums"); + if (!zip || !sums) { + console.error("Usage: node scripts/amo-submit.mjs --zip --sums [--id ]"); + process.exit(2); + } + const id = arg("--id") ?? JSON.parse(readFileSync(join(ROOT, "extension/manifest.json"), "utf8")).browser_specific_settings.gecko.id; + try { + const hash = await submit({ zip, sums, id, apiKey: process.env.WEB_EXT_API_KEY, apiSecret: process.env.WEB_EXT_API_SECRET }); + console.log(`Submitted ${basename(zip)} to Firefox Add-ons as ${id}, unchanged: SHA-256 ${hash}, as in ${basename(sums)}.`); + } catch (e) { + console.error(`amo-submit: ${e.message}`); + process.exitCode = 1; + } +} diff --git a/scripts/base-file.sh b/scripts/base-file.sh new file mode 100755 index 0000000..7d1f870 --- /dev/null +++ b/scripts/base-file.sh @@ -0,0 +1,38 @@ +#!/usr/bin/env bash +# SPDX-FileCopyrightText: 2026 ScriptKittyOS and the Kotiko contributors +# SPDX-License-Identifier: Apache-2.0 +# +# Copies files as they are at a trusted commit into a folder, so that a pull request's +# required checks run with the base branch's settings and scripts, not the ones the pull +# request brings (security review C-01). Otherwise a pull request could add a secret and, +# in the same change, allow it in .gitleaks.toml, or accept its own vulnerable dependency in +# osv-scanner.toml, and pass. Its edits to these files take effect once it is merged. +# +# scripts/base-file.sh ... +# +# Each (relative to the repository's top) is written to /. A path the +# commit doesn't have stops the script, so a check never runs without its settings. CI runs +# the base commit's own copy of this script (.github/workflows/ci.yml), not the pull +# request's. + +set -euo pipefail + +die() { + echo "base-file: $*" >&2 + exit 1 +} + +[ $# -ge 3 ] || die "usage: base-file.sh ..." +commit="$1" +dir="$2" +shift 2 + +git cat-file -e "$commit^{commit}" 2>/dev/null || die "$commit isn't a commit in this repository" +for path in "$@"; do + case "/$path/" in + //* | */../* | */./* | *//*) die "$path isn't a plain path inside the repository" ;; + esac + [ "$(git cat-file -t "$commit:$path" 2>/dev/null || true)" = blob ] || die "$commit has no file $path" + mkdir -p "$dir/$(dirname "$path")" + git cat-file blob "$commit:$path" >"$dir/$path" +done diff --git a/scripts/check-security-gate.mjs b/scripts/check-security-gate.mjs index 7d9bad7..91778c1 100755 --- a/scripts/check-security-gate.mjs +++ b/scripts/check-security-gate.mjs @@ -3,29 +3,56 @@ // SPDX-License-Identifier: Apache-2.0 // The store uploads wait for the pre-release security review (slice 54; DECISIONS -// 2026-10-02). The review ends with docs/security/review-.md and a line saying the gate -// is closed: +// 2026-10-02). The review ends with docs/security/review-vX.Y.Z-rc.N.md, in which the lead +// closes the gate with one line of exactly this form, on a line of its own, above the +// "## Appendix" heading under which the reviewers' reports are appended unchanged: // -// Gate: closed 2026-11-02 by , fixes confirmed on v0.3.0-rc.2 +// Gate: closed 2026-11-02 by , fixes confirmed on v1.0.0-rc.2 // -// This check passes when the tagged tree has at least one such report. The release workflow -// runs it before asking for the `release` environment's approval, so no store upload can -// happen before the first review is done. Later releases that touch permissions, messaging, -// secrets, the server's API or the bot reopen the review by hand: a box on the release -// checklist (docs/stores.md). +// The candidate named is one of the reviewed version's (1.0.0 here). Above the appendix the +// report has exactly one line starting "Gate:", so a "Gate: open" line keeps it open. Lines +// below the appendix count for nothing: a reviewer's report may quote the phrase (security +// review C-05). // -// node scripts/check-security-gate.mjs [--dir docs/security] +// This check passes when the tagged tree has a closed report. With --version X.Y.Z (the +// release workflow passes the tag's), a review opened for that version must itself be +// closed; a version without its own review is gated by the earlier ones. The release +// workflow runs it before asking for the `release` environment's approval, so no store +// upload can happen before the first review is done. Later releases that touch +// permissions, messaging, secrets, the server's API or the bot reopen the review by hand: a +// box on the release checklist (docs/stores.md). +// +// node scripts/check-security-gate.mjs [--dir docs/security] [--version X.Y.Z] import { readdirSync, readFileSync } from "node:fs"; import { dirname, join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; const ROOT = resolve(dirname(fileURLToPath(import.meta.url)), ".."); -export const REPORT_RE = /^review-v\d+\.\d+\.\d+(?:-rc\.\d+)?\.md$/; -export const CLOSED_RE = /^\s*(?:[-*]\s*)?(?:\*\*)?Gate:(?:\*\*)?\s*closed\b/im; +export const REPORT_RE = /^review-v(\d+\.\d+\.\d+)(?:-rc\.\d+)?\.md$/; +export const CLOSED_RE = /^Gate: closed (\d{4}-\d{2}-\d{2}) by (.+), fixes confirmed on (v\d+\.\d+\.\d+-rc\.\d+)$/; +export const APPENDIX_RE = /^## Appendix\b/; +// Any line that reads as a gate line, however it is dressed up, so that only one may exist. +const ANY_GATE_RE = /^[\s>*_\-+#]*gate[*_\s]*:/i; + +const realDate = (d) => !Number.isNaN(Date.parse(`${d}T00:00:00Z`)) && new Date(`${d}T00:00:00Z`).toISOString().startsWith(d); + +/** The lead's closing line in a report's text, or null while the gate is open. */ +export function gateLine(text) { + const lines = text.split(/\r?\n/); + const end = lines.findIndex((l) => APPENDIX_RE.test(l)); + if (end < 0) return null; + const gates = lines.slice(0, end).filter((l) => ANY_GATE_RE.test(l)); + if (gates.length !== 1) return null; + const m = gates[0].match(CLOSED_RE); + if (!m || !realDate(m[1])) return null; + return { date: m[1], by: m[2], candidate: m[3] }; +} -/** Report files in dir whose text has a "Gate: closed" line. */ -export function closedReviews(dir) { +const core = (tag) => tag.replace(/^v/, "").replace(/-rc\.\d+$/, ""); + +/** Report files in dir whose gate is closed; with version, as that version's release sees them. */ +export function closedReviews(dir, { version } = {}) { let names; try { names = readdirSync(dir); @@ -33,19 +60,36 @@ export function closedReviews(dir) { if (e.code === "ENOENT") return []; throw e; } - return names.filter((n) => REPORT_RE.test(n)).sort().filter((n) => CLOSED_RE.test(readFileSync(join(dir, n), "utf8"))); + let reports = names.filter((n) => REPORT_RE.test(n)).sort(); + if (version && reports.some((n) => n.match(REPORT_RE)[1] === version)) { + reports = reports.filter((n) => n.match(REPORT_RE)[1] === version); + } + return reports.filter((n) => { + const line = gateLine(readFileSync(join(dir, n), "utf8")); + return line !== null && core(line.candidate) === n.match(REPORT_RE)[1]; + }); } if (process.argv[1] && resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { - const i = process.argv.indexOf("--dir"); - const dir = i > 0 ? resolve(process.argv[i + 1]) : join(ROOT, "docs/security"); - const closed = closedReviews(dir); + const arg = (name) => { + const i = process.argv.indexOf(name); + return i > 0 ? process.argv[i + 1] : undefined; + }; + const dir = arg("--dir") ? resolve(arg("--dir")) : join(ROOT, "docs/security"); + const version = arg("--version"); + if (version !== undefined && !/^\d+\.\d+\.\d+(?:-rc\.\d+)?$/.test(version)) { + console.error(`check-security-gate: --version takes X.Y.Z, not ${version}`); + process.exit(2); + } + const closed = closedReviews(dir, { version: version && core(version) }); if (closed.length) { console.log(`Security review closed: ${closed.join(", ")}`); } else { console.error( "check-security-gate: no closed security review. Store uploads wait for slice 54's review: " + - "docs/security/review-.md with a line starting \"Gate: closed\".", + "docs/security/review-vX.Y.Z-rc.N.md with, above its \"## Appendix\" heading, the one line " + + "\"Gate: closed YYYY-MM-DD by , fixes confirmed on vX.Y.Z-rc.N\"" + + (version ? `, and if ${core(version)} has its own review, that one closed.` : "."), ); process.exitCode = 1; } diff --git a/scripts/check-versions.mjs b/scripts/check-versions.mjs index 11df0e9..6728cd7 100644 --- a/scripts/check-versions.mjs +++ b/scripts/check-versions.mjs @@ -1,9 +1,10 @@ // SPDX-FileCopyrightText: 2026 ScriptKittyOS and the Kotiko contributors // SPDX-License-Identifier: Apache-2.0 -// Fails when the extension, the server and the release manifest disagree on the version. +// Fails when the extension, the server and the release manifest disagree on the version, +// and, from 1.0.0 on, when SECURITY.md still calls Kotiko pre-1.0 (security review B-08). // No dependencies, so CI can run it before `npm ci`. -import { readFileSync } from "node:fs"; +import { existsSync, readFileSync } from "node:fs"; const read = (p) => readFileSync(new URL(`../${p}`, import.meta.url), "utf8"); @@ -19,3 +20,10 @@ if (distinct.size !== 1 || distinct.has(undefined)) { console.error("Versions differ. Change them together (release-please does this on release)."); process.exit(1); } + +const [version] = distinct; +const security = new URL("../SECURITY.md", import.meta.url); +if (Number(version.split(".")[0]) >= 1 && existsSync(security) && /Kotiko is (?:still )?(?:before|pre-?) ?1\.0\b/i.test(readFileSync(security, "utf8"))) { + console.error(`SECURITY.md still says Kotiko is before 1.0, but the version is ${version}. Update its supported versions.`); + process.exit(1); +} diff --git a/scripts/tag-release.sh b/scripts/tag-release.sh index 493812d..32ae129 100755 --- a/scripts/tag-release.sh +++ b/scripts/tag-release.sh @@ -17,8 +17,8 @@ # commit that changed .release-please-manifest.json; the three version files agree; # CHANGELOG.md is ready (scripts/release-notes.mjs --check). Then `git tag -s` with your own # SSH or GPG signing key (git config user.signingkey, gpg.format), checks the signature -# against .github/allowed_signers exactly as the workflow will, and pushes the tag. The push -# starts the release workflow. +# (against .github/allowed_signers) and the name inside the signed tag exactly as the +# workflow will, and pushes the tag. The push starts the release workflow. set -euo pipefail @@ -94,6 +94,12 @@ else fi "${sign[@]}" tag -s "$tag" -m "Kotiko $version" +# The name inside the signed object must be the tag's own (verify-tag.sh checks it too): the +# workflow refuses a release whose signed object carries another name (security review C-02). +if [ "$(git cat-file tag "refs/tags/$tag" | sed -n '/^$/q; s/^tag //p')" != "$tag" ]; then + git tag -d "$tag" >/dev/null + die "the new tag's signed object isn't named $tag; it was deleted, nothing was pushed" +fi if ! bash scripts/verify-tag.sh "$tag" --main "$remote/main" --rc-branch "$remote/$rc_branch"; then git tag -d "$tag" >/dev/null die "the new tag didn't verify (is your key in .github/allowed_signers?); it was deleted, nothing was pushed" diff --git a/scripts/verify-tag.sh b/scripts/verify-tag.sh index 7481e0e..9594221 100755 --- a/scripts/verify-tag.sh +++ b/scripts/verify-tag.sh @@ -10,6 +10,9 @@ # 2. It is an annotated tag signed with an SSH key listed in the allowed signers file # (default .github/allowed_signers), or with a GPG key from the keys file (default # .github/release-keys.asc). Unsigned tags, lightweight tags and other keys fail. +# The name inside the signed tag object is the tag's name: the signature covers the +# object, not the ref, so a candidate's object pushed again as refs/tags/vX.Y.Z would +# otherwise pass as a release (security review C-02). # 3. Its commit is on main (--main, default origin/main). A release candidate may also sit on # release-please's release branch (--rc-branch), whose head carries the next version. # 4. extension/manifest.json, server/mix.exs and .release-please-manifest.json at that commit @@ -17,7 +20,8 @@ # # The release workflow passes the signers and keys files from main, not from the tagged # commit, so a tag can't vouch for itself. Prints version=… and prerelease=… (also to -# $GITHUB_OUTPUT when set). Needs git 2.34+ and OpenSSH 8.2+ for SSH signatures. +# $GITHUB_OUTPUT when set, with commit=…, which the build compares with the commit it checks +# out). Needs git 2.34+ and OpenSSH 8.2+ for SSH signatures. set -euo pipefail @@ -79,6 +83,10 @@ elif grep -q -- '-----BEGIN PGP SIGNATURE-----' <<<"$object"; then else die "$tag isn't signed; release tags are signed with git tag -s" fi +# The header ends at the first empty line; the message and signature follow it. +signed_name="$(sed -n '/^$/q; s/^tag //p' <<<"$object")" +[ "$signed_name" = "$tag" ] || + die "refs/tags/$tag holds a tag object signed as ${signed_name:-nothing}, not $tag; a release tag is signed under its own name" # 3. Where the commit is. commit="$(git rev-parse "$tag^{commit}")" diff --git a/slices/28-privacy-and-store-readiness/SPEC.md b/slices/28-privacy-and-store-readiness/SPEC.md index 863a1b8..c908205 100644 --- a/slices/28-privacy-and-store-readiness/SPEC.md +++ b/slices/28-privacy-and-store-readiness/SPEC.md @@ -625,6 +625,14 @@ captioned screenshots from the real extension on `test/fixtures/pages/store-arti `BACKGROUND_SERVICE_WORKER_IGNORED`, which slice 30's Firefox build can clear. - Artwork (05) and uploads (30). +### 2026-10-06: Firefox data collection (slice 54, finding C-11) + +The manifest now declares `data_collection_permissions: {required: ["authenticationInfo"]}`: +Mozilla's policy counts the learner's own key, sent to the service they chose, as +authentication information leaving the browser, and the Chrome form already said so. The +`required: ["none"]` above is the original decision. Making it optional (asked for when a +key is first saved) is a follow-up. + ## Acceptance criteria - [ ] The privacy policy exists at both locations, matches `inventory.md` row by row, and diff --git a/slices/30-release-pipeline/SPEC.md b/slices/30-release-pipeline/SPEC.md index 9e1a3dc..68c6bd7 100644 --- a/slices/30-release-pipeline/SPEC.md +++ b/slices/30-release-pipeline/SPEC.md @@ -482,6 +482,16 @@ Built 2026-10-05. What runs where: attest v4.2.2 `1e69f48`, download-artifact v8.0.1 `3e5f45b`. Checkout, setup-node, setup-beam and upload-artifact use CI's existing pins. +### 2026-10-06: security review (slice 54) changes + +- Firefox Add-ons now gets the release's own zip, unchanged, through web-ext's AMO client + (`scripts/amo-submit.mjs`, which checks it against SHA256SUMS first) instead of + `web-ext sign --source-dir`, which repacked it (finding C-06). Sections 5 and 8's mentions of + `web-ext sign` describe the original design. +- `verify-tag.sh` also checks the name inside the signed tag object (C-02), the build checks + out exactly the verified commit, and the release build runs property tests with fixed seeds + (C-08). The store gate accepts only the exact `Gate: closed` line above `## Appendix` (C-05). + ## Acceptance criteria - [ ] Pushing the signed tag for a merged release PR (section 11) creates the GitHub diff --git a/slices/54-pre-release-security-review/SPEC.md b/slices/54-pre-release-security-review/SPEC.md index 4767fb1..d367ae1 100644 --- a/slices/54-pre-release-security-review/SPEC.md +++ b/slices/54-pre-release-security-review/SPEC.md @@ -146,7 +146,8 @@ The lead reviewer reads the three reports after all are in, then for each findin The output is `docs/security/review-.md`: scope and commit, the reviewers and how they were kept independent, a table of every finding (confirmed, rejected or accepted, with severity and the fix's pull request), the threat-area coverage matrix (area by reviewer: -checked with proof, not checked), and the three original reports appended unchanged. While +checked with proof, not checked), and the three original reports appended unchanged under +a `## Appendix` heading (nothing of the lead's own goes below that heading). While the repository is private it holds full details; when it goes public, unfixed low findings are summarised without exploit steps (SECURITY.md's disclosure rules, 03). @@ -159,10 +160,15 @@ are summarised without exploit steps (SECURITY.md's disclosure rules, 03). with a reason, and as issues. - A fourth reviewer, fresh and independent like the first three, reruns every confirmed finding's proof against `-rc.2` and confirms it no longer works, then the lead closes the - gate with a line in the report and a box on 30's release checklist. The line starts - `Gate: closed` (for example `Gate: closed 2026-11-02 by , fixes confirmed on - v0.3.0-rc.2`): 30's release workflow looks for it before any store upload - (`scripts/check-security-gate.mjs`). + gate with a line in the report and a box on 30's release checklist. The line has exactly + this form, on a line of its own, above the `## Appendix` heading: + `Gate: closed YYYY-MM-DD by , fixes confirmed on vX.Y.Z-rc.N` (for example + `Gate: closed 2026-11-02 by Ayla Croft, fixes confirmed on v1.0.0-rc.2`), naming a + candidate of the reviewed version. Until then the report says `Gate: open`; the part + above the appendix has only one line starting `Gate:`. 30's release workflow checks for + the closed line before any store upload (`scripts/check-security-gate.mjs`): anything + else, including the phrase inside an appended report, keeps the gate open (security + review C-05). - Confirmed findings feed 53 §4.7's assurance case and, for issues in shipped code, 53 §4.2's vulnerability process. diff --git a/store/chrome-web-store.md b/store/chrome-web-store.md index 5a2488d..25c15ea 100644 --- a/store/chrome-web-store.md +++ b/store/chrome-web-store.md @@ -76,7 +76,7 @@ on the device, per the User Data FAQ): | Personally identifiable information | No | Kotiko asks for no name, email or account. | | Health information | No | | | Financial and payment information | No | | -| Authentication information | **Yes** | The learner's own API key for the AI service they chose, and the access key of their own Kotiko server. Stored in the extension's IndexedDB, where web pages and content scripts can't read them, and sent only to the service each belongs to. | +| Authentication information | **Yes** | The learner's own API key for the AI service they chose, and the access key of their own Kotiko server. Stored in the extension's IndexedDB, where web pages and content scripts can't read them, and sent only to the service each belongs to. (Firefox's manifest declares the same: `data_collection_permissions` `required: ["authenticationInfo"]`, [firefox-amo.md](firefox-amo.md).) | | Personal communications | No | | | Location | No | | | Web history | No | Kotiko doesn't record the pages you visit. It checks the current page's address in the browser to honour sites the learner paused; it stores only the host names the learner chose to pause, and sends none. | diff --git a/store/firefox-amo.md b/store/firefox-amo.md index 8c80ef2..acb00a2 100644 --- a/store/firefox-amo.md +++ b/store/firefox-amo.md @@ -4,7 +4,7 @@ What Kotiko declares to Firefox and types into addons.mozilla.org, from [slice 28](../slices/28-privacy-and-store-readiness/SPEC.md) §4. Data statements come from [`docs/privacy/inventory.md`](../docs/privacy/inventory.md). -Primary sources (checked 2026-10-05): +Primary sources (checked 2026-10-05; the data declaration again 2026-10-06): [Add-on policies](https://extensionworkshop.com/documentation/publish/add-on-policies/), [Firefox built-in data collection consent](https://extensionworkshop.com/documentation/develop/firefox-builtin-data-consent/), [MDN `browser_specific_settings`](https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/browser_specific_settings), @@ -19,7 +19,7 @@ Primary sources (checked 2026-10-05): "gecko": { "id": "kotiko@scriptkittyos.com", "strict_min_version": "140.0", - "data_collection_permissions": { "required": ["none"] } + "data_collection_permissions": { "required": ["authenticationInfo"] } }, "gecko_android": { "strict_min_version": "142.0" } } @@ -28,17 +28,32 @@ Primary sources (checked 2026-10-05): - **`data_collection_permissions`** is required for every new add-on since 3 November 2025, and Firefox reads it from version 140 on desktop (142 on Android). 140 is an ESR, so ESR users are covered; that is why `strict_min_version` is 140. -- **`required: ["none"]`.** Out of the box Kotiko transmits nothing: a word typed with its - meaning ("hola = hello") never leaves the browser except, for languages with word stress, - the word alone to Wiktionary for its pronunciation. What a learner chooses to send - afterwards is their own input to a service they set up with their own key: the text they - type to add a word goes to that AI service. Mozilla's categories (personally identifying, - health, financial, authentication, personal communications, location, browsing - activity, website content, website activity, search terms, bookmarks, technical and - interaction) describe data about the user and their browsing; text the user types to - look up is none of them, and page content never leaves the browser. Confidence: medium. - The first submission asks the reviewer in its notes (below); if AMO disagrees, the - declaration changes in the same release as the policy. +- **`required: ["authenticationInfo"]`** (security review C-11, 2026-10-06). The + learner's own API key for the AI service they chose, and the access token of their own + Kotiko server, leave the browser: each is sent in the `Authorization` header to the + service it belongs to. Mozilla counts that. Its policy defines data transmission as "any + data collected, used, transferred, shared, or handled outside the add-on or the local + browser" ([Add-on policies](https://extensionworkshop.com/documentation/publish/add-on-policies/) + §6), with no exception for a service the user chose, and its `authenticationInfo` + category is "passwords, usernames, personal identification numbers (PINs), security + questions, and registration information" ([built-in consent](https://extensionworkshop.com/documentation/develop/firefox-builtin-data-consent/)). + A key is a credential, so `none` would misstate it. This matches the Chrome form's + "Authentication information: Yes" ([chrome-web-store.md](chrome-web-store.md#data-usage); + `store-readiness.test.mjs` checks the two agree). Nothing goes to ScriptKittyOS. + - **Required, not optional, for now.** An optional permission is granted only when the + extension asks with `browser.permissions.request({ data_collection: ["authenticationInfo"] })` + in a click handler; until Kotiko asks when a key is first saved, "Connect OpenRouter" + is pressed or a server is paired (and keeps the key on the device if the learner + says no), declaring it optional would claim a consent Kotiko never collects. Moving to + `optional` with that request is the better fit, since lookups without a key work. + - **Everything else stays undeclared.** A word typed with its meaning ("hola = hello") + never leaves the browser except, for languages with word stress, the word alone to + Wiktionary for its pronunciation; with an AI service set up, the text typed into + Kotiko's add box goes to that service. Text the learner types into Kotiko to look up + is none of Mozilla's other categories (the nearest, `searchTerms`, is "search terms + entered into search engines or the browser"), and page content never leaves the + browser. Confidence: medium. The first submission asks the reviewer in its notes + (below); if AMO disagrees, the declaration changes in the same release as the policy. - **No `optional` list yet.** Slice 33's "Learn this in…" would send text selected on a page, which is `websiteContent`; it adds `"optional": ["websiteContent"]` and asks with `browser.permissions.request({ data_collection: ["websiteContent"] })` the first time the @@ -94,13 +109,15 @@ a zip of the repository at the release tag (slice 30). > word with its meaning in your browser's language (for example "hola = hello"), press > "Make it my first word", then "Try it on a page". > -> Data collection: we declared `required: ["none"]`. Page content never leaves the -> browser. When a learner sets up an AI service with their own key, the text they type into -> Kotiko's add box is sent to that service to look the word up; for languages with word -> stress, the word alone is sent to en.wiktionary.org for its pronunciation. We read -> neither as one of the data categories, since both are the learner's own input to the -> feature, but we'd like your view: if you consider either one `websiteContent` or another -> category, we'll declare it. +> Data collection: we declared `required: ["authenticationInfo"]`: a learner who sets up +> an AI service or their own Kotiko server gives Kotiko that service's key, which is sent +> only to that service (never to us). Page content never leaves the browser. With an AI +> service set up, the text the learner types into Kotiko's add box is sent to that service +> to look the word up; for languages with word stress, the word alone is sent to +> en.wiktionary.org for its pronunciation. We read neither as one of the data categories, +> since both are the learner's own input to the feature, but we'd like your view: if you +> consider either one `searchTerms`, `websiteContent` or another category, we'll declare +> it. > > `extension/spec/spec.js` is generated from `spec/` in our public repository > (https://github.com/ScriptKittyOS/kotiko) by `spec/tools/sync-extension.mjs`; nothing is diff --git a/test/unit/amo-submit.test.mjs b/test/unit/amo-submit.test.mjs new file mode 100644 index 0000000..cc9b44e --- /dev/null +++ b/test/unit/amo-submit.test.mjs @@ -0,0 +1,106 @@ +// SPDX-FileCopyrightText: 2026 ScriptKittyOS and the Kotiko contributors +// SPDX-License-Identifier: Apache-2.0 + +// Security review C-06: Firefox Add-ons got a zip that `web-ext sign --source-dir` built +// again, not the release's attested zip. scripts/amo-submit.mjs checks the zip against +// SHA256SUMS and submits that exact file through web-ext's AMO v5 client. Here a fake AMO +// server on 127.0.0.1 records what arrives; nothing reaches the network. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { createHash } from "node:crypto"; +import { createServer } from "node:http"; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { strToU8, zipSync } from "fflate"; +import { listedHash, submit } from "../../scripts/amo-submit.mjs"; + +const ROOT = join(dirname(fileURLToPath(import.meta.url)), "../.."); +const hex = (buf) => createHash("sha256").update(buf).digest("hex"); + +// A fake AMO: upload, validation (done at once), new version. Records each request. +async function fakeAmo(t) { + const seen = []; + const server = createServer(async (req, res) => { + const chunks = []; + for await (const c of req) chunks.push(c); + const body = Buffer.concat(chunks); + const entry = { method: req.method, path: req.url, auth: req.headers.authorization }; + seen.push(entry); + const json = (obj) => { + res.writeHead(200, { "content-type": "application/json" }); + res.end(JSON.stringify(obj)); + }; + if (req.method === "POST" && req.url === "/api/v5/addons/upload/") { + const form = await new Response(body, { headers: { "content-type": req.headers["content-type"] } }).formData(); + const file = form.get("upload"); + entry.channel = form.get("channel"); + entry.filename = file.name; + entry.bytes = Buffer.from(await file.arrayBuffer()); + return json({ uuid: "u-1" }); + } + if (req.method === "GET" && req.url === "/api/v5/addons/upload/u-1/") return json({ uuid: "u-1", processed: true, valid: true }); + if (req.method === "PUT" && decodeURIComponent(req.url) === "/api/v5/addons/addon/kotiko@scriptkittyos.com/") { + entry.json = JSON.parse(body.toString("utf8")); + return json({ guid: "kotiko@scriptkittyos.com", version: { id: 7, edit_url: "https://example.invalid/edit" } }); + } + res.writeHead(404).end("{}"); + }); + await new Promise((r) => server.listen(0, "127.0.0.1", r)); + t.after(() => server.close()); + return { seen, url: `http://127.0.0.1:${server.address().port}/api/v5/` }; +} + +function release(t) { + const dir = mkdtempSync(join(tmpdir(), "kotiko-amo-test-")); + t.after(() => rmSync(dir, { recursive: true, force: true })); + const zip = join(dir, "kotiko-firefox-9.9.9.zip"); + // web-ext reads the zip's entries (for its upload cache), so a real zip. + writeFileSync(zip, zipSync({ "manifest.json": strToU8('{"version":"9.9.9"}\n') }, { mtime: new Date("2026-10-06T00:00:00Z") })); + const sums = join(dir, "SHA256SUMS"); + writeFileSync(sums, `${hex(readFileSync(zip))} kotiko-firefox-9.9.9.zip\n${"0".repeat(64)} kotiko-chrome-9.9.9.zip\n`); + return { dir, zip, sums }; +} + +test("the zip AMO receives is the release's zip, byte for byte, as a listed version", async (t) => { + const amo = await fakeAmo(t); + const { zip, sums } = release(t); + const hash = await submit({ zip, sums, id: "kotiko@scriptkittyos.com", apiKey: "user:1:2", apiSecret: "s".repeat(64), amoBaseUrl: amo.url }); + const upload = amo.seen.find((r) => r.path === "/api/v5/addons/upload/"); + assert.ok(upload, "uploaded"); + assert.equal(hex(upload.bytes), hex(readFileSync(zip)), "the same bytes, not a rebuilt zip"); + assert.equal(hash, hex(readFileSync(zip))); + assert.equal(upload.filename, "kotiko-firefox-9.9.9.zip"); + assert.equal(upload.channel, "listed"); + assert.match(upload.auth, /^JWT [\w-]+\.[\w-]+\.[\w-]+$/); + const version = amo.seen.find((r) => r.method === "PUT"); + assert.deepEqual(version.json, { version: { upload: "u-1" } }); +}); + +test("a zip that doesn't match SHA256SUMS, or isn't in it, is never uploaded", async (t) => { + const amo = await fakeAmo(t); + const { dir, zip, sums } = release(t); + const args = { sums, id: "kotiko@scriptkittyos.com", apiKey: "k", apiSecret: "s", amoBaseUrl: amo.url }; + writeFileSync(zip, "repacked"); + await assert.rejects(submit({ ...args, zip }), /has SHA-256 [0-9a-f]{64}, but .*SHA256SUMS says/); + const other = join(dir, "kotiko-firefox-1.0.0.zip"); + writeFileSync(other, "x"); + await assert.rejects(submit({ ...args, zip: other }), /isn't listed in/); + await assert.rejects(submit({ ...args, zip, apiKey: "" }), /WEB_EXT_API_KEY/); + assert.deepEqual(amo.seen, [], "no request reached AMO"); +}); + +test("listedHash reads sha256sum's text and binary forms", () => { + const h = "a".repeat(64); + assert.equal(listedHash(`${h} x.zip\n`, "x.zip"), h); + assert.equal(listedHash(`${h} *x.zip\n`, "x.zip"), h); + assert.equal(listedHash(`${h} x.zip.bak\n`, "x.zip"), null); +}); + +test("the release workflow submits with amo-submit, not web-ext sign", () => { + const wf = readFileSync(join(ROOT, ".github/workflows/release.yml"), "utf8"); + const job = wf.slice(wf.indexOf("\n publish-firefox:")); + assert.match(job, /node scripts\/amo-submit\.mjs --zip "dist\/kotiko-firefox-\$VERSION\.zip" --sums dist\/SHA256SUMS/); + assert.doesNotMatch(job, /web-ext sign|unzip/); +}); diff --git a/test/unit/check-versions.test.mjs b/test/unit/check-versions.test.mjs new file mode 100644 index 0000000..17a8a70 --- /dev/null +++ b/test/unit/check-versions.test.mjs @@ -0,0 +1,36 @@ +// SPDX-FileCopyrightText: 2026 ScriptKittyOS and the Kotiko contributors +// SPDX-License-Identifier: Apache-2.0 + +// scripts/check-versions.mjs: from 1.0.0 on, SECURITY.md no longer calls Kotiko pre-1.0 +// (security review B-08). Runs a copy in a throwaway folder. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import { copyFileSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const ROOT = join(dirname(fileURLToPath(import.meta.url)), "../.."); + +function tree(t, version, security) { + const dir = mkdtempSync(join(tmpdir(), "kotiko-versions-")); + t.after(() => rmSync(dir, { recursive: true, force: true })); + for (const d of ["scripts", "extension", "server"]) mkdirSync(join(dir, d)); + copyFileSync(join(ROOT, "scripts/check-versions.mjs"), join(dir, "scripts/check-versions.mjs")); + writeFileSync(join(dir, ".release-please-manifest.json"), JSON.stringify({ ".": version })); + writeFileSync(join(dir, "extension/manifest.json"), JSON.stringify({ version })); + writeFileSync(join(dir, "server/mix.exs"), ` def project do\n [\n version: "${version}",\n`); + if (security !== undefined) writeFileSync(join(dir, "SECURITY.md"), security); + return spawnSync("node", [join(dir, "scripts/check-versions.mjs")], { encoding: "utf8" }); +} + +test("from 1.0.0 on, SECURITY.md can't still say Kotiko is before 1.0", (t) => { + const stale = "## Supported versions\n\nKotiko is before 1.0. Only the latest minor release gets security fixes.\n"; + assert.equal(tree(t, "0.2.0", stale).status, 0, "true before 1.0"); + const r = tree(t, "1.0.0", stale); + assert.equal(r.status, 1); + assert.match(r.stderr, /SECURITY\.md still says Kotiko is before 1\.0/); + assert.equal(tree(t, "1.0.0", readFileSync(join(ROOT, "SECURITY.md"), "utf8")).status, 0, "today's SECURITY.md is fine at 1.0.0"); + assert.equal(tree(t, "1.0.0").status, 0, "no SECURITY.md (other tests' throwaway repositories)"); +}); diff --git a/test/unit/pr-checks-base.test.mjs b/test/unit/pr-checks-base.test.mjs new file mode 100644 index 0000000..15febe9 --- /dev/null +++ b/test/unit/pr-checks-base.test.mjs @@ -0,0 +1,128 @@ +// SPDX-FileCopyrightText: 2026 ScriptKittyOS and the Kotiko contributors +// SPDX-License-Identifier: Apache-2.0 + +// Security review C-01: the required `secrets` and `osv-scanner` checks read their allowlists +// (.gitleaks.toml, .gitleaksignore, osv-scanner.toml) and their check scripts from the pull +// request's own tree, so a pull request could add a secret and allow it in the same change. +// On a pull request they now come from the base commit (scripts/base-file.sh, the base's own +// copy). These tests run the helper in a throwaway repository where a pull request adds a +// token and edits every allowlist and check script to let it through, and check that the +// workflows use the base's copies. With CI's gitleaks version installed (GITLEAKS=/path/to/gitleaks, +// or on PATH) they also scan that pull request both ways. +import { test, before, after } from "node:test"; +import assert from "node:assert/strict"; +import { execFileSync, spawnSync } from "node:child_process"; +import { copyFileSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { randomBytes } from "node:crypto"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const ROOT = join(dirname(fileURLToPath(import.meta.url)), "../.."); +const TRUSTED = [".gitleaks.toml", ".gitleaksignore", "scripts/check-forbidden-files.mjs", "scripts/check-dco.mjs"]; + +let base; +let env; +before(() => { + base = mkdtempSync(join(tmpdir(), "kotiko-pr-checks-")); + env = { + PATH: process.env.PATH, + HOME: base, + GIT_CONFIG_GLOBAL: "/dev/null", + GIT_CONFIG_NOSYSTEM: "1", + GIT_AUTHOR_NAME: "Test", GIT_AUTHOR_EMAIL: "test@example.com", + GIT_COMMITTER_NAME: "Test", GIT_COMMITTER_EMAIL: "test@example.com", + LC_ALL: "C", + }; +}); +after(() => rmSync(base, { recursive: true, force: true })); + +const run = (cwd, cmd, args) => spawnSync(cmd, args, { cwd, env, encoding: "utf8" }); +const git = (cwd, ...args) => execFileSync("git", args, { cwd, env, encoding: "utf8" }).trim(); + +// A fake GitHub token, made at run time so that this file holds no secret-shaped text. +const fakeToken = () => "ghp_" + randomBytes(64).toString("base64").replace(/[^A-Za-z1-9]/g, "").slice(0, 36); + +// main with the real settings and scripts, then a pull request that adds a token and edits +// .gitleaks.toml, .gitleaksignore and both check scripts to let it through. +let n = 0; +function repo() { + const dir = join(base, `case${++n}`); + mkdirSync(join(dir, "scripts"), { recursive: true }); + git(dir, "init", "-q", "-b", "main"); + for (const f of [...TRUSTED, "scripts/base-file.sh"]) copyFileSync(join(ROOT, f), join(dir, f)); + git(dir, "add", "-A"); + git(dir, "commit", "-q", "-m", "chore: start"); + const baseSha = git(dir, "rev-parse", "HEAD"); + git(dir, "switch", "-q", "-c", "pr"); + writeFileSync(join(dir, "leak.js"), `const t = "${fakeToken()}";\n`); + writeFileSync(join(dir, ".gitleaks.toml"), readFileSync(join(dir, ".gitleaks.toml"), "utf8") + "paths = ['''leak\\.js''']\n"); + writeFileSync(join(dir, ".gitleaksignore"), "# tidy\n"); + for (const s of ["scripts/check-forbidden-files.mjs", "scripts/check-dco.mjs"]) writeFileSync(join(dir, s), "process.exit(0);\n"); + writeFileSync(join(dir, "scripts/base-file.sh"), "#!/usr/bin/env bash\nexit 0\n"); + git(dir, "add", "-A"); + git(dir, "commit", "-q", "-m", "tidy gitleaks config"); + return { dir, baseSha }; +} + +test("base-file copies the base commit's settings and scripts, not the pull request's edits", () => { + const { dir, baseSha } = repo(); + const out = join(dir, "..", `trusted${n}`); + const r = run(dir, "bash", [join(ROOT, "scripts/base-file.sh"), baseSha, out, ...TRUSTED]); + assert.equal(r.status, 0, r.stderr); + for (const f of TRUSTED) { + assert.equal(readFileSync(join(out, f), "utf8"), readFileSync(join(ROOT, f), "utf8"), `${f} is the base's copy`); + assert.notEqual(readFileSync(join(out, f), "utf8"), readFileSync(join(dir, f), "utf8"), `${f} isn't the pull request's`); + } +}); + +test("base-file refuses a missing file, a path outside the repository and an unknown commit", () => { + const { dir, baseSha } = repo(); + const out = join(dir, "..", `refused${n}`); + const script = join(ROOT, "scripts/base-file.sh"); + assert.match(run(dir, "bash", [script, baseSha, out, "leak.js"]).stderr, /has no file leak\.js/); + assert.match(run(dir, "bash", [script, baseSha, out, "../x"]).stderr, /isn't a plain path/); + assert.match(run(dir, "bash", [script, baseSha, out, "/etc/passwd"]).stderr, /isn't a plain path/); + assert.match(run(dir, "bash", [script, baseSha, out, "scripts/../.gitleaks.toml"]).stderr, /isn't a plain path/); + assert.match(run(dir, "bash", [script, "0".repeat(40), out, ".gitleaks.toml"]).stderr, /isn't a commit/); + assert.notEqual(run(dir, "bash", [script, baseSha, out]).status, 0, "usage"); +}); + +// The gitleaks version CI pins; another build may read its settings differently. +const PINNED = readFileSync(join(ROOT, ".github/workflows/ci.yml"), "utf8").match(/GITLEAKS_VERSION: "([^"]+)"/)[1]; +const gitleaks = [process.env.GITLEAKS, "gitleaks"].find((bin) => bin && spawnSync(bin, ["version"], { encoding: "utf8" }).stdout?.trim() === PINNED); +// GITLEAKS set (as in CI's secrets job) means the scan must run; otherwise it is optional. +test("a pull request that allows its own token in .gitleaks.toml and .gitleaksignore still fails the scan", { skip: !gitleaks && !process.env.GITLEAKS && `gitleaks ${PINNED} not installed (set GITLEAKS)` }, () => { + assert.ok(gitleaks, `GITLEAKS=${process.env.GITLEAKS} isn't gitleaks ${PINNED}`); + const { dir, baseSha } = repo(); + const scan = (...args) => run(dir, gitleaks, ["git", "--log-opts=HEAD", "--redact", "--exit-code", "1", ...args, "."]); + // What CI ran before: the pull request's own settings let its token through. + assert.equal(scan("--config", ".gitleaks.toml").status, 0, "the bypass the review found"); + // What CI runs now (ci.yml, job secrets): the base's settings, the tree's copies removed + // (gitleaks always reads the scanned folder's .gitleaksignore too), inline allows ignored. + const out = join(dir, "..", `scan${n}`); + assert.equal(run(dir, "bash", [join(ROOT, "scripts/base-file.sh"), baseSha, out, ...TRUSTED]).status, 0); + rmSync(join(dir, ".gitleaks.toml")); + rmSync(join(dir, ".gitleaksignore")); + const r = scan("--config", join(out, ".gitleaks.toml"), "--gitleaks-ignore-path", join(out, ".gitleaksignore"), "--ignore-gitleaks-allow"); + assert.equal(r.status, 1, r.stderr); + assert.match(r.stderr, /leaks found: 1/); +}); + +test("on a pull request, CI's required checks use the base commit's settings and scripts", () => { + const ci = readFileSync(join(ROOT, ".github/workflows/ci.yml"), "utf8"); + const secrets = ci.slice(ci.indexOf("\n secrets:"), ci.indexOf("\n server:")); + assert.match(secrets, /git show "\$ref:scripts\/base-file\.sh"/, "the base's own copy of the helper"); + for (const f of TRUSTED) assert.ok(secrets.includes(` ${f}`), `${f} comes from the base`); + assert.match(secrets, /rm -f \.gitleaks\.toml \.gitleaksignore/); + assert.match(secrets, /--config "\$RUNNER_TEMP\/trusted\/\.gitleaks\.toml" --gitleaks-ignore-path "\$RUNNER_TEMP\/trusted\/\.gitleaksignore" --ignore-gitleaks-allow/); + assert.match(secrets, /node "\$RUNNER_TEMP\/trusted\/scripts\/check-forbidden-files\.mjs"/); + assert.match(secrets, /node "\$RUNNER_TEMP\/trusted\/scripts\/check-dco\.mjs"/); + assert.doesNotMatch(secrets, /node scripts\/check-/, "no check script from the pull request's tree"); + + const osv = readFileSync(join(ROOT, ".github/workflows/dependency-scan.yml"), "utf8"); + assert.match(osv, /git show "\$ref:scripts\/base-file\.sh"/); + assert.match(osv, / osv-scanner\.toml\n/); + assert.match(osv, /scan source --config "\$RUNNER_TEMP\/trusted\/osv-scanner\.toml"/); + assert.doesNotMatch(osv, /--config osv-scanner\.toml/); +}); diff --git a/test/unit/release-notes.test.mjs b/test/unit/release-notes.test.mjs index d8d05cd..64c5ed6 100644 --- a/test/unit/release-notes.test.mjs +++ b/test/unit/release-notes.test.mjs @@ -113,6 +113,9 @@ test("the store gate opens only with a closed security review report", (t) => { writeFileSync(join(dir, "review-latest.md"), "Gate: closed\n"); writeFileSync(join(dir, "review-v0.3.0-rc.2.md"), "# Review\n\nThe gate is not closed yet; it says Gate: closed only at the start of a line.\n"); assert.deepEqual(closedReviews(dir), []); - writeFileSync(join(dir, "review-v0.3.0-rc.2.md"), "# Review\n\n**Gate: closed** 2026-11-02 by the lead; fixes confirmed on v0.3.0-rc.2\n"); + // The exact form, above the appendix (more cases in security-gate.test.mjs). + writeFileSync(join(dir, "review-v0.3.0-rc.2.md"), "# Review\n\n**Gate: closed** 2026-11-02 by the lead; fixes confirmed on v0.3.0-rc.2\n\n## Appendix\n"); + assert.deepEqual(closedReviews(dir), []); + writeFileSync(join(dir, "review-v0.3.0-rc.2.md"), "# Review\n\nGate: closed 2026-11-02 by the lead, fixes confirmed on v0.3.0-rc.2\n\n## Appendix\n"); assert.deepEqual(closedReviews(dir), ["review-v0.3.0-rc.2.md"]); }); diff --git a/test/unit/release-tags.test.mjs b/test/unit/release-tags.test.mjs index 8135808..33e2ae3 100644 --- a/test/unit/release-tags.test.mjs +++ b/test/unit/release-tags.test.mjs @@ -243,3 +243,20 @@ test("GPG-signed tags verify only against the keys in the release keys file", { assert.match(verify(work, "v0.3.0", "--keys", join(work, "other.asc")).stderr, /not in .*other\.asc/); assert.match(verify(work, "v0.3.0", "--keys", join(work, "none.asc")).stderr, /no release keys/); }); + +// Reviewer C-02 (slice 54): a ref named like a release that holds another tag's signed object +// (here the candidate v9.9.9-rc.1 pushed again as refs/tags/v9.9.9) would otherwise verify as +// a final release, because the signature covers the object, not the ref's name. +test("a signed candidate's tag object under a release's name is refused", () => { + const { work } = setup(); + writeVersion(work, "9.9.9"); + git(work, "commit", "-q", "-am", "chore(main): release 9.9.9"); + git(work, "push", "-q", "origin", "main"); + tagWith(work, "v9.9.9-rc.1", "release"); + assert.equal(verify(work, "v9.9.9-rc.1").status, 0, "the candidate itself verifies"); + git(work, "update-ref", "refs/tags/v9.9.9", git(work, "rev-parse", "refs/tags/v9.9.9-rc.1")); + const r = verify(work, "v9.9.9"); + assert.equal(r.status, 1, r.stdout); + assert.doesNotMatch(r.stdout, /prerelease=false/); + assert.match(r.stderr, /signed as v9\.9\.9-rc\.1, not v9\.9\.9/); +}); diff --git a/test/unit/security-gate.test.mjs b/test/unit/security-gate.test.mjs new file mode 100644 index 0000000..ecb6f88 --- /dev/null +++ b/test/unit/security-gate.test.mjs @@ -0,0 +1,91 @@ +// SPDX-FileCopyrightText: 2026 ScriptKittyOS and the Kotiko contributors +// SPDX-License-Identifier: Apache-2.0 + +// Slice 54 §6: the store gate opens only on the lead's own "Gate: closed" line, in its one +// exact form, above the report's "## Appendix" heading. Security review C-05: before, any line +// starting with "Gate: closed" anywhere in a review-v*.md opened it, including inside the +// reviewers' reports appended below and in "**Gate:** closed? No" notes. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { closedReviews, gateLine } from "../../scripts/check-security-gate.mjs"; + +const ROOT = join(dirname(fileURLToPath(import.meta.url)), "../.."); +const CLOSED = "Gate: closed 2026-11-02 by Ayla Croft, fixes confirmed on v1.0.0-rc.2"; + +function gateDir(t, files) { + const dir = mkdtempSync(join(tmpdir(), "kotiko-gate-")); + t.after(() => rmSync(dir, { recursive: true, force: true })); + for (const [name, text] of Object.entries(files)) writeFileSync(join(dir, name), text); + return dir; +} +const report = (lead, appendix = "") => `# Security review: v1.0.0-rc.1\n\n${lead}\n\n## Appendix: the reviewers' reports, unchanged\n\n${appendix}\n`; + +// Reviewer C's three texts (test/security/poc/gate-false-positive.sh at v1.0.0-rc.1). +test("C-05: an open review stays open, whatever its appended reports or notes say", (t) => { + for (const text of [ + "Gate: open (fixes pending on rc.2)\n", + "Gate: open\n\n## Appendix: reviewer A report (unchanged)\n- Gate: closed is what the lead writes at the end; not yet.\n", + "**Gate:** closed? No: two medium findings are still open.\n", + ]) { + const dir = gateDir(t, { "review-v1.0.0-rc.1.md": text }); + assert.deepEqual(closedReviews(dir), [], text); + } +}); + +test("only the exact closed line, above the appendix, closes the gate", (t) => { + assert.deepEqual(closedReviews(gateDir(t, { "review-v1.0.0-rc.1.md": report(CLOSED) })), ["review-v1.0.0-rc.1.md"]); + for (const lead of [ + "Gate: open", + `**${CLOSED}**`, + `- ${CLOSED}`, + `${CLOSED}.`, + "Gate: closed 2026-11-02 by Ayla Croft", + "Gate: closed 2026-13-02 by Ayla Croft, fixes confirmed on v1.0.0-rc.2", + "Gate: closed 2026-11-02 by Ayla Croft, fixes confirmed on v1.0.0", + "Gate: closed soon, by Ayla Croft, fixes confirmed on v1.0.0-rc.2", + `Gate: open\n${CLOSED}`, + `${CLOSED}\nGate: open again: a fix regressed`, + ]) { + assert.deepEqual(closedReviews(gateDir(t, { "review-v1.0.0-rc.1.md": report(lead) })), [], lead); + } + // The line in an appended report counts for nothing; neither does a report without an appendix heading. + assert.deepEqual(closedReviews(gateDir(t, { "review-v1.0.0-rc.1.md": report("Gate: open", CLOSED) })), []); + assert.deepEqual(closedReviews(gateDir(t, { "review-v1.0.0-rc.1.md": `# Review\n\n${CLOSED}\n` })), []); + // The candidate the fixes were confirmed on is a candidate of the reviewed version. + assert.deepEqual(closedReviews(gateDir(t, { "review-v1.1.0-rc.1.md": report(CLOSED) })), []); +}); + +test("gateLine reads the date, the lead and the candidate", () => { + assert.deepEqual(gateLine(report(CLOSED)), { date: "2026-11-02", by: "Ayla Croft", candidate: "v1.0.0-rc.2" }); + assert.equal(gateLine(report("Gate: open")), null); +}); + +test("a review opened for the version being released must be closed; older closed reviews don't stand in", (t) => { + const older = "Gate: closed 2026-11-02 by Ayla Croft, fixes confirmed on v1.0.0-rc.2"; + const dir = gateDir(t, { + "review-v1.0.0-rc.1.md": report(older), + "review-v1.1.0-rc.1.md": report("Gate: open"), + }); + assert.deepEqual(closedReviews(dir, { version: "1.0.1" }), ["review-v1.0.0-rc.1.md"], "no review for 1.0.1: the first review gates it"); + assert.deepEqual(closedReviews(dir, { version: "1.1.0" }), [], "1.1.0's own review is open"); + writeFileSync(join(dir, "review-v1.1.0-rc.1.md"), report("Gate: closed 2027-01-10 by Ayla Croft, fixes confirmed on v1.1.0-rc.3")); + assert.deepEqual(closedReviews(dir, { version: "1.1.0" }), ["review-v1.1.0-rc.1.md"]); +}); + +test("the command line: --version, and the exit code", (t) => { + const dir = gateDir(t, { "review-v1.0.0-rc.1.md": report("Gate: open", CLOSED) }); + const gate = (...args) => spawnSync("node", ["scripts/check-security-gate.mjs", "--dir", dir, ...args], { cwd: ROOT, encoding: "utf8" }); + let r = gate("--version", "1.0.0"); + assert.equal(r.status, 1); + assert.match(r.stderr, /no closed security review/); + writeFileSync(join(dir, "review-v1.0.0-rc.1.md"), report(CLOSED)); + r = gate("--version", "1.0.0"); + assert.equal(r.status, 0, r.stderr); + assert.match(r.stdout, /Security review closed: review-v1\.0\.0-rc\.1\.md/); + assert.equal(gate("--version", "v1.0.0").status, 2, "a version, not a tag"); +}); diff --git a/test/unit/store-readiness.test.mjs b/test/unit/store-readiness.test.mjs index 84d6be0..b2c821e 100644 --- a/test/unit/store-readiness.test.mjs +++ b/test/unit/store-readiness.test.mjs @@ -75,17 +75,43 @@ describe("permissions (slice 28 §3)", () => { }); describe("Firefox and the content security policy (slice 28 §4, §5)", () => { - test("the manifest declares no data collection, needs Firefox 140, and states the extension pages' policy", () => { + test("the manifest declares its data collection, needs Firefox 140, and states the extension pages' policy", () => { const m = manifest(); const gecko = m.browser_specific_settings.gecko; assert.equal(gecko.id, "kotiko@scriptkittyos.com"); assert.equal(gecko.strict_min_version, "140.0", "the first desktop Firefox that reads data_collection_permissions"); - assert.deepEqual(gecko.data_collection_permissions, { required: ["none"] }); + assert.deepEqual(gecko.data_collection_permissions, { required: ["authenticationInfo"] }); assert.equal(m.browser_specific_settings.gecko_android.strict_min_version, "142.0", "the first Android Firefox that reads it"); // Chrome's default, written out: Firefox's own adds upgrade-insecure-requests, which // would turn a LAN or Tailscale http:// server into an https:// one that isn't there. assert.deepEqual(m.content_security_policy, { extension_pages: "script-src 'self'; object-src 'self'" }); - assert.match(read("store/firefox-amo.md"), /"required": \["none"\]/); + assert.match(read("store/firefox-amo.md"), /"required": \["authenticationInfo"\]/); + }); + + // Security review C-11: the Chrome form ticked authentication information while Firefox's + // manifest declared no data collection. Chrome's form counts data "handled" even on the + // device (website content: yes) and Firefox's only data that leaves the browser, so the + // categories that leave it must match: here, the learner's own key and server token. + test("the Firefox declaration and the Chrome form agree on what leaves the browser", () => { + const declared = manifest().browser_specific_settings.gecko.data_collection_permissions; + const firefox = new Set([...(declared.required ?? []), ...(declared.optional ?? [])].filter((c) => c !== "none")); + const chrome = Object.fromEntries(table(CWS, "### Data usage").map(([name, tick]) => [name, /\*\*Yes\*\*/.test(tick)])); + // Chrome's categories and Firefox's for the same data; website content is read on the + // device only (Chrome: handled; Firefox: not transmitted), so it isn't compared. + const SAME = { + "Personally identifiable information": "personallyIdentifyingInfo", + "Health information": "healthInfo", + "Financial and payment information": "financialAndPaymentInfo", + "Authentication information": "authenticationInfo", + "Personal communications": "personalCommunications", + Location: "locationInfo", + "Web history": "browsingActivity", + }; + for (const [cws, gecko] of Object.entries(SAME)) { + assert.ok(cws in chrome, `store/chrome-web-store.md has no "${cws}" row`); + assert.equal(firefox.has(gecko), chrome[cws], `Chrome "${cws}" is ${chrome[cws] ? "Yes" : "No"}, Firefox ${gecko} is ${firefox.has(gecko) ? "" : "not "}declared`); + } + assert.ok(!(declared.required ?? []).includes("none") || firefox.size === 0, "none stands alone"); }); test("the http:// warning: only addresses whose traffic crosses a network unencrypted", async () => {