chore: pin Actions SHAs, safe deps, bind dashboard identity - #60
Merged
Conversation
Keep the public reference repo current with SHA-pinned CI (matching corporate-site-harness#20), safe patch/minor bumps, and server-side operator/tenant binding on decide/appeal so clients cannot spoof #28. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
🟡 Changes recommended
A few workflow lines have misleading actions/checkout version comments, and the new tenant-binding introduces redundant DB reads on decide/appeal paths.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR tightens the ops dashboard mutation contract by binding decidedBy and tenantId to server-side identity (env-backed for the shared bearer token), while also applying safe dependency bumps and pinning GitHub Actions by immutable SHAs.
Changes:
- Bind
/api/exceptions/:id/decideand/api/exceptions/:id/appealtoDASHBOARD_OPERATOR_ID/DASHBOARD_TENANT_ID, ignoring client-suppliedby/tenantId, and rejecting cross-tenant mutations with 403. - Add a focused regression test (
dashboard-identity.test.ts) covering “client by ignored” and “cross-tenant denied”. - Pin Actions by commit SHA and update safe deps (
@hono/node-server,hono,globals), plus update docs/ADR/env examples accordingly.
File summaries
| File | Description |
|---|---|
apps/api/src/app.ts |
Derives dashboard identity from env, enforces tenant binding on decide/appeal, updates response shape. |
apps/console/src/main.tsx |
Stops sending by in decide requests (server-derived identity). |
test/dashboard-identity.test.ts |
Adds tests asserting server-derived decidedBy/tenant and 403 on cross-tenant operations. |
.env.example |
Documents DASHBOARD_OPERATOR_ID / DASHBOARD_TENANT_ID defaults and intent. |
SECURITY.md |
Documents identity binding + cross-tenant denial behavior and updated deps. |
README.md |
Updates security posture blurb to reflect identity binding. |
CONTEXT.md |
Notes decide/appeal identity binding in shared mode context. |
docs/adr/0013-console-hitl-kill-resume.md |
Records the identity-binding decision in ADR. |
package.json |
Bumps @hono/node-server and globals ranges; updates overrides to keep @hono/node-server pinned. |
package-lock.json |
Locks updated versions (@hono/node-server 2.0.12, hono 4.13.7, globals 17.12.0). |
apps/api/package.json |
Bumps API package deps (@hono/node-server, hono). |
.github/workflows/ci.yml |
Pins setup-node to SHA v7.0.0; keeps checkout pinned. |
.github/workflows/codeql.yml |
Pins CodeQL actions to v4.37.9 SHAs; keeps checkout pinned. |
.github/workflows/scorecard.yml |
Pins Scorecard to v2.4.4 + CodeQL upload-sarif v4.37.9; keeps checkout pinned. |
.github/workflows/docker.yml |
Keeps checkout pinned (comment updated in this PR). |
.github/workflows/deploy.yml |
Keeps checkout pinned (comment updated in this PR). |
Review details
Suppressed comments (2)
.github/workflows/ci.yml:40
- The pinned SHA is for actions/checkout v4, but the inline comment claims v7.0.1 (which does not exist for actions/checkout). This is misleading for auditing and future maintenance—update the comment to match the actual major version being pinned.
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
apps/api/src/app.ts:171
- This introduces an extra DB read: bindExceptionTenant() loads the exception row to check tenant, and appealException() loads the same row again. Consider moving the tenant check into appealException (or adding a core helper that accepts a preloaded exception) to avoid double-selects on this path.
const bound = await bindExceptionTenant(company, c.req.param("id"), identity.tenantId);
if (bound.status !== 200) return c.json({ error: bound.error }, bound.status);
const out = await appealException(
company,
bound.ex.id,
identity.operatorId,
- Files reviewed: 15/16 changed files
- Comments generated: 6
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+145
to
+149
| const bound = await bindExceptionTenant(company, c.req.param("id"), identity.tenantId); | ||
| if (bound.status !== 200) return c.json({ error: bound.error }, bound.status); | ||
| const out = await decideException( | ||
| company, | ||
| c.req.param("id"), | ||
| bound.ex.id, |
| steps: | ||
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 | ||
| - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6 | ||
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 |
| language: [javascript-typescript] | ||
| steps: | ||
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 | ||
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 |
| timeout-minutes: 15 | ||
| steps: | ||
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 | ||
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 |
| timeout-minutes: 15 | ||
| steps: | ||
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 | ||
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 |
|
|
||
| steps: | ||
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v4 | ||
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
v4.37.9(cdf488f595d80d6e07e03d4674febd5ab45fa938), Scorecardv2.4.4, setup-nodev7.0.0.@hono/node-server2.0.12,hono4.13.7,globals17.12.0 (lock).decidedByand tenant fromDASHBOARD_OPERATOR_ID/DASHBOARD_TENANT_ID(token-bound defaults). Client-suppliedby/tenantIdare ignored; cross-tenant attempts return 403. Still a shared static token — not a production SaaS / OIDC claim.Supersedes Dependabot #55, #53, #52, #51, #50, #48, #47 after merge. Skips majors: vite 6→8 (#56), zod 3→4 (#54), nanoid 5→6 (#49).
Change type
Checklist
./scripts/harness/verify.shis green locally../scripts/harness/adversarial.shis green locally (or N/A for docs-only).better-sqlite3introduced.*.db,.env, ordist/staged.autoApproveExceptiondefault-off unless a test/CI opt-in is intentional.docs/adr/.npm run screenshots).Test plan
./scripts/harness/verify.sh→verify: ok(22 tests)./scripts/harness/adversarial.sh→adversarial: ok(AUTH still 401 without bearer)test/dashboard-identity.test.ts: clientbyignored; cross-tenant decide/appeal 403Made with Cursor