Skip to content

ci: pin actions to SHA and run full test suite on PRs #131

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
zeevmoney wants to merge 4 commits into
base: main
Choose a base branch
from
Open

ci: pin actions to SHA and run full test suite on PRs #131

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f438bc3
ci: pin actions to SHA and run full test suite on PRs
zeevmoney Jun 28, 2026
5e29bcc
ci: run unit/integration/module-imports on PR, defer e2e to next PR
zeevmoney Jun 28, 2026
f5dffbf
ci: run on all pull requests, not only those targeting main
zeevmoney Jun 29, 2026
fc2529b
ci: pin PDP connection to IPv4 (127.0.0.1) in the backend test run
zeevmoney Jun 29, 2026
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
200 changes: 173 additions & 27 deletions .github/workflows/ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,38 +3,184 @@ name: CI
on:
push:
branches: [ main ]
# Run on every PR regardless of base branch so stacked PRs get CI too.
pull_request:
branches: [ main ]

env:
PDP_API_KEY: test
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

permissions:
contents: read

jobs:
test-and-lint:
lint-and-build:
runs-on: ubuntu-latest
steps:
- name: Checkout code
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
persist-credentials: false
- name: Setup Node.js
uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0
with:
node-version: 22
cache: 'yarn'
- name: Install dependencies
run: yarn install --frozen-lockfile
- name: Run linting
run: yarn lint
- name: Build project
run: yarn build

test:
needs: lint-and-build
runs-on: ubuntu-latest

strategy:
fail-fast: false
matrix:
node-version: [18, 20]

node-version: [20, 22]
env:
# Backend suite runs only for same-repo events AND when the provisioning
# secret is available. Fork PRs and secret-less runs (e.g. Dependabot)
# fall back to the no-backend suite.
RUN_BACKEND: ${{ (github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository) && secrets.PROJECT_API_KEY != '' }}

@dshoen619 dshoen619 Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🟡 This gate will block Dependabot (latent today).

Dependabot PRs are same-repo (head.repo.full_name == github.repository → true) but get no access to regular secrets, so RUN_BACKEND evaluates to false and the "Require backend secret" step (line 67) hits exit 1. There's no .github/dependabot.yml yet, so it's unreachable today — but it breaks the moment Dependabot (or any same-repo bot without the secret) is enabled. Exempt the actor:

Suggested change
RUN_BACKEND: ${{ (github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository) && secrets.PROJECT_API_KEY != '' }}
RUN_BACKEND: ${{ (github.event_name == 'push' || (github.event.pull_request.head.repo.full_name == github.repository && github.actor != 'dependabot[bot]')) && secrets.PROJECT_API_KEY != '' }}

Apply the same && github.actor != 'dependabot[bot]' to the gate if: on line 67.

steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Setup Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node-version }}
cache: 'yarn'

- name: Install dependencies
run: yarn install --frozen-lockfile

- name: Run linting
run: yarn lint

- name: Build project
run: yarn build

- name: Run all tests
run: yarn test
- name: Checkout code
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
persist-credentials: false

- name: Setup Node.js ${{ matrix.node-version }}
uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0
with:
node-version: ${{ matrix.node-version }}
cache: 'yarn'

- name: Install dependencies
run: yarn install --frozen-lockfile

# Fail fast if a same-repo run lacks the backend secret, so the full
# suite can't be silently downgraded to unit-only and still report green.
# Forks (different head repo) intentionally skip this and fall back below.
- name: Require backend secret on same-repo runs
if: ${{ (github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository) && env.RUN_BACKEND != 'true' }}
run: |
echo "::error::PROJECT_API_KEY is not set for a same-repo run; integration/e2e would be silently skipped. Failing instead of reporting a misleading green." >&2
exit 1

# ---------- Backend path (same-repo only) ----------
- name: Provision temp Permit env
id: provision
if: env.RUN_BACKEND == 'true'
env:
PROJECT_API_KEY: ${{ secrets.PROJECT_API_KEY }}
PROJECT_ID: 7f55831d77c642739bc17733ab0af138
ENV_KEY: node-sdk-ci-${{ github.run_id }}-${{ github.run_attempt }}-n${{ matrix.node-version }}
run: |
response=$(curl -sS -X POST \
"https://api.permit.io/v2/projects/${PROJECT_ID}/envs" \
-H "Authorization: Bearer ${PROJECT_API_KEY}" \
-H 'Content-Type: application/json' \
-d "{\"key\":\"${ENV_KEY}\",\"name\":\"${ENV_KEY}\"}")
env_id=$(printf '%s' "$response" | jq -r '.id // empty')
if [ -z "$env_id" ]; then
echo "Failed to create env (key=${ENV_KEY})." >&2

@dshoen619 dshoen619 Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Low — error responses are swallowed. curl -sS exits 0 on HTTP 4xx/5xx and only .id presence is checked, so a 401/429 surfaces as a bare "Failed to create env" with no context. Echo the response (safe — PROJECT_API_KEY is in the request header, not the body):

Suggested change
echo "Failed to create env (key=${ENV_KEY})." >&2
echo "Failed to create env (key=${ENV_KEY}). Response: ${response}" >&2

Same applies to the "Fetch env API key" step (line 106).

exit 1
fi
echo "env_id=${env_id}" >> "$GITHUB_OUTPUT"

- name: Fetch env API key
id: fetch_key
if: env.RUN_BACKEND == 'true'
env:
PROJECT_API_KEY: ${{ secrets.PROJECT_API_KEY }}
PROJECT_ID: 7f55831d77c642739bc17733ab0af138
ENV_ID: ${{ steps.provision.outputs.env_id }}
run: |
response=$(curl -sS -X GET \
"https://api.permit.io/v2/api-key/${PROJECT_ID}/${ENV_ID}" \
-H "Authorization: Bearer ${PROJECT_API_KEY}")
env_api_key=$(printf '%s' "$response" | jq -r '.secret // empty')
if [ -z "$env_api_key" ]; then
echo "Failed to fetch env API key." >&2
exit 1
fi
# Mask BEFORE writing to the env file so it is redacted everywhere.
echo "::add-mask::${env_api_key}"
echo "ENV_API_KEY=${env_api_key}" >> "$GITHUB_ENV"

- name: Start local PDP
if: env.RUN_BACKEND == 'true'
env:
ENV_API_KEY: ${{ env.ENV_API_KEY }}
run: |
docker run -d --name pdp -p 7766:7000 \
-e PDP_API_KEY="$ENV_API_KEY" \
-e PERMIT_API_KEY="$ENV_API_KEY" \
permitio/pdp-v2:latest
Comment on lines +118 to +121

@dshoen619 dshoen619 Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🟡 permitio/pdp-v2:latest is a mutable tag — inconsistent with the SHA-pinning discipline this PR otherwise applies. A silent PDP image update changes CI behavior with no code change, and a compromised image would receive ENV_API_KEY at runtime. Pin to a digest (permitio/pdp-v2@sha256:<digest>) and bump it intentionally. (The publish workflow uses the same mutable tag.)


- name: Wait for PDP to be ready
if: env.RUN_BACKEND == 'true'
run: |
for i in $(seq 1 60); do
if curl -sf http://127.0.0.1:7766/healthy >/dev/null; then
echo "PDP ready after ${i} attempt(s)"
exit 0
fi
sleep 2
done
echo "PDP did not become healthy in time; dumping logs:" >&2
docker logs pdp || true
exit 1

- name: Run full test suite (backend)
if: env.RUN_BACKEND == 'true'
env:
PDP_API_KEY: ${{ env.ENV_API_KEY }}
PERMIT_API_KEY: ${{ env.ENV_API_KEY }}
API_TIER: prod
# Force IPv4: Node resolves `localhost` to ::1 first, but the runner's
# Docker IPv6 publish refuses connections, so PDP checks would hit
# ECONNREFUSED. 127.0.0.1 pins the working IPv4 path.
PDP_URL: http://127.0.0.1:7766
run: yarn test:ci:full

@dshoen619 dshoen619 Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Tied to the package.json comment: because test:ci:full doesn't include the e2e suites, this step runs unit + integration + module-imports only. The integration/endpoints suite (src/tests/endpoints/test-environments.spec.ts) makes control-plane API calls only and never calls permit.check() — so the dockerized PDP, the /healthy wait, and the IPv4 PDP_URL pin above are provisioned but never actually queried by anything that runs here. Once the e2e wiring is fixed in package.json, this all becomes load-bearing.


# Dump PDP diagnostics whenever the backend path fails. The SDK reports
# PDP connection errors with no HTTP response, so the PDP side is otherwise
# invisible; this captures container state and logs to tell a transient
# restart/readiness blip apart from a crashed/exited container.
- name: Dump PDP diagnostics on failure
if: ${{ failure() && env.RUN_BACKEND == 'true' }}
run: |
echo "::group::docker ps -a"
docker ps -a --filter name=pdp || true
echo "::endgroup::"
echo "::group::PDP container state"
docker inspect -f \
'status={{.State.Status}} restartCount={{.RestartCount}} exitCode={{.State.ExitCode}} oomKilled={{.State.OOMKilled}} startedAt={{.State.StartedAt}} finishedAt={{.State.FinishedAt}}' \
pdp || true
echo "::endgroup::"
echo "::group::docker logs pdp"
docker logs pdp || true
echo "::endgroup::"
echo "::group::curl -sv http://localhost:7766/healthy"
curl -sv http://localhost:7766/healthy || true
Comment on lines +167 to +168

@dshoen619 dshoen619 Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🟡 Diagnostic uses localhost, defeating the IPv4 workaround. The readiness loop deliberately uses 127.0.0.1 (the whole point of the PDP_URL pin), but this on-failure diagnostic uses localhost → resolves to ::1 → reports connection-refused even when the PDP is healthy on IPv4, i.e. it misleads exactly when you need it.

Suggested change
curl -sv http://localhost:7766/healthy || true
curl -sv http://127.0.0.1:7766/healthy || true

echo "::endgroup::"

# ---------- No-backend path (forks / secret-less) ----------
- name: Run no-backend test suite
if: env.RUN_BACKEND != 'true'
run: yarn test:ci:unit

# ---------- Cleanup (always, even on failure/cancel) ----------
- name: Delete temp Permit env
if: ${{ always() && steps.provision.outputs.env_id != '' }}

@dshoen619 dshoen619 Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Note (acknowledged in the PR): cleanup keys off steps.provision.outputs.env_id, so if cancel-in-progress kills the provision step after the env is created server-side but before env_id is written to $GITHUB_OUTPUT, the env leaks (up to 2/run with the matrix). The deferred sweeper is the right fix — worth prioritizing since the leak rate scales with PR/push volume.

env:
PROJECT_API_KEY: ${{ secrets.PROJECT_API_KEY }}
PROJECT_ID: 7f55831d77c642739bc17733ab0af138
ENV_ID: ${{ steps.provision.outputs.env_id }}
run: |
curl -sS -X DELETE \
"https://api.permit.io/v2/projects/${PROJECT_ID}/envs/${ENV_ID}" \
-H "Authorization: Bearer ${PROJECT_API_KEY}" || true
9 changes: 6 additions & 3 deletions .github/workflows/node_sdk_publish.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,14 @@ jobs:
contents: read
id-token: write # Required for npm Trusted Publishing (OIDC)
steps:
# Safe: this workflow makes only local git commits (docs/version bump); nothing is pushed.
- name: Checkout code
uses: actions/checkout@v4
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
persist-credentials: false

@dshoen619 dshoen619 Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

persist-credentials: false ✅ — confirmed safe: this workflow makes only local commits and never git pushes. One pre-existing corollary (out of scope, just noting): because nothing is pushed, the git commit -m "update tsdoc" on line 43 has always been discarded at job end — a dead write. Either add a push or drop the commit step.


- name: Setup Node.js
uses: actions/setup-node@v4
uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0
with:
node-version: '20'
registry-url: 'https://registry.npmjs.org'
Expand Down Expand Up @@ -51,7 +54,7 @@ jobs:
}')

# Extract the new env id
echo "ENV_ID=$(echo "$response" | jq -r '.id')" >> $GITHUB_ENV
echo "ENV_ID=$(echo "$response" | jq -r '.id')" >> "$GITHUB_ENV"

echo "New env ID: $ENV_ID"

Expand Down
2 changes: 2 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@
"test:e2e:rbac": "run-s build && ava --verbose build/tests/e2e/rbac.e2e.spec.js",
"test:e2e:rebac": "run-s build && ava --verbose build/tests/e2e/rebac.e2e.spec.js",
"test:e2e:local_facts": "run-s build && ava --verbose build/tests/e2e/local_facts.e2e.spec.js",
"test:ci:unit": "run-s test:unit test:module-imports",
"test:ci:full": "run-s test",

@dshoen619 dshoen619 Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔴 Blocking — test:ci:full never runs the e2e suites (the PR's headline goal).

run-s test → run-s test:*, and npm-run-all treats : as a path separator: a single * does not cross it. Verified empirically with this repo's npm-run-all@4.1.5:

run-s test:*  →  test:unit, test:integration, test:module-imports     # test:e2e:* NOT matched

So test:ci:full runs unit + integration + module-imports only — test:e2e:rbac/rebac/local_facts silently never execute, and the "flaky e2e sleeps" risk is moot because they don't run. Enumerate explicitly:

Suggested change
"test:ci:full": "run-s test",
"test:ci:full": "run-s test:unit test:integration test:module-imports test:e2e:rbac test:e2e:rebac test:e2e:local_facts",

⚠️ Do not "fix" this with run-s test:** — ** does cross :, so it would match test:ci:full/test:ci:unit and infinite-loop.

Two adjacent notes (lines 34–35, outside this hunk): test:integration and test:module-imports pass their ** globs unquoted, unlike test:unit — works only because those dirs are flat today; add a subdirectory and the shell pre-expands the glob and silently drops the top-level spec. Quote them like test:unit. Also, each test:* re-runs build, so test:ci:full rebuilds 3×.

"check-cli": "run-s test diff-integration-tests check-integration-tests",
"check-integration-tests": "run-s check-integration-test:*",
"diff-integration-tests": "mkdir -p diff && rm -rf diff/test && cp -r test diff/test && rm -rf diff/test/test-*/.git && cd diff && git init --quiet && git add -A && git commit --quiet --no-verify --allow-empty -m 'WIP' && echo '\\n\\nCommitted most recent integration test output in the \"diff\" directory. Review the changes with \"cd diff && git diff HEAD\" or your preferred git diff viewer.'",
Expand Down
Loading