Repository navigation
fix: secure and release SpaceApp 0.1.5 - #11
Conversation
|
Warning Review limit reached
Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThe release updates CI action pins, package versions, release documentation, frontend tooling, runtime security paths, Windows Docker process execution, and published-container attestation verification. It also adds regression coverage for screenshot clipping, redirects, command validation, rate-limit ordering, and container evidence loading. ChangesRuntime security and platform behavior
Published container evidence verification
Release tooling and version updates
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant VerificationCLI
participant DockerBuildx
participant Registry
VerificationCLI->>DockerBuildx: Inspect image manifest
DockerBuildx-->>VerificationCLI: Return platform and attestation descriptors
VerificationCLI->>Registry: Inspect attestation manifests by digest
Registry-->>VerificationCLI: Return attestation layers
VerificationCLI->>VerificationCLI: Evaluate platform and predicate evidence
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
apps/api/tests/route-rate-limits.test.ts (1)
49-67: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest the runtime ordering instead of relying solely on source offsets.
Lines [52-65] only verify literal text order in
app.ts; they do not prove that Fastify registers the limiter and hooks in the same runtime scope or execution order. A refactor could preserve these strings while changing plugin encapsulation or behavior. Add an executable integration assertion using the app instance/request path, optionally retaining this as a lightweight structural guard.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/tests/route-rate-limits.test.ts` around lines 49 - 67, Replace or supplement the source-text offset checks in the test named “registers the global limiter before request authentication and lifecycle hooks” with an executable integration assertion using the Fastify app instance and request path. Verify at runtime that the global rate limiter is registered before authentication and lifecycle hooks execute, while optionally retaining the existing structural check as a lightweight guard.scripts/verify-published-containers.mjs (2)
60-67: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSurface stderr in the thrown inspection errors.
inspectRawdiscardsresult.stderron failure, making CI failures harder to diagnose (e.g. auth errors vs. missing image vs. network issues all collapse into"Could not inspect ${image}.").♻️ Include stderr in the error
if (result.status !== 0) { - throw new Error(`Could not inspect ${image}.`); + throw new Error(`Could not inspect ${image}: ${result.stderr?.trim() || "unknown error"}`); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/verify-published-containers.mjs` around lines 60 - 67, Update inspectRaw’s nonzero-status error path to include result.stderr in the thrown error alongside the image identifier, while preserving the existing JSON parse error handling.
25-39: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAttestation lookup only inspects the first matching entry per platform digest.
.find(...)at line 25 stops at the first attestation-manifest whosevnd.docker.reference.digestmatches the platform digest. If a platform ever has more than one attestation manifest attached for the same digest (e.g. attestations added by a separate tool/step after the initial buildx build), predicates in the second manifest are never inspected, risking a false "missing SBOM/provenance" blocker even though the evidence exists.♻️ Aggregate predicates across all matching attestation manifests
- const attestation = (Array.isArray(attestations) ? attestations : []).find( - (entry) => - entry?.descriptor?.annotations?.["vnd.docker.reference.type"] === - "attestation-manifest" && - entry?.descriptor?.annotations?.["vnd.docker.reference.digest"] === - platformManifest.digest - ); - const predicates = new Set( - (Array.isArray(attestation?.manifest?.layers) - ? attestation.manifest.layers - : [] - ) - .map((layer) => layer?.annotations?.["in-toto.io/predicate-type"]) - .filter((value) => typeof value === "string") - ); + const matchingAttestations = (Array.isArray(attestations) ? attestations : []).filter( + (entry) => + entry?.descriptor?.annotations?.["vnd.docker.reference.type"] === + "attestation-manifest" && + entry?.descriptor?.annotations?.["vnd.docker.reference.digest"] === + platformManifest.digest + ); + const predicates = new Set( + matchingAttestations + .flatMap((attestation) => + Array.isArray(attestation?.manifest?.layers) ? attestation.manifest.layers : [] + ) + .map((layer) => layer?.annotations?.["in-toto.io/predicate-type"]) + .filter((value) => typeof value === "string") + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/verify-published-containers.mjs` around lines 25 - 39, Update the attestation processing around the current find call to inspect every attestation-manifest whose vnd.docker.reference.digest matches platformManifest.digest, rather than stopping at the first match. Aggregate the predicate types from all matching manifests into the existing predicates Set, while preserving the current filtering of valid layer annotations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Line 23: Disable persisted checkout credentials by adding the checkout
action’s with.persist-credentials setting as false at all listed sites:
.github/workflows/ci.yml:23-23, .github/workflows/containers.yml:41-41,
.github/workflows/platform-matrix.yml:34-34,
.github/workflows/release.yml:32-32, 50-50, 141-141, 219-219, 296-296, 322-322,
and .github/workflows/security.yml:27-27, 110-110, 120-120, 148-148. Apply the
setting to each corresponding actions/checkout invocation without changing the
surrounding workflow behavior.
In `@docs/public-release.md`:
- Around line 110-111: Update the rollback guidance in the release documentation
to reference failed 0.1.5 staging artifacts instead of the stale 0.1.4
reference, unless both versions are intentionally required; keep the instruction
to preserve artifacts and avoid deleting or overwriting exact version tags.
In `@packages/run-spaceapp/src/prerequisites.mjs`:
- Around line 180-197: The PowerShell -Command invocations do not reliably
receive executable paths and arguments through $args. Update both installer
execution sites in prerequisites.mjs, including the install flow and Docker
Desktop start flow, to use a supported parameterized script-block, encoded
command, or properly terminated -File transport; then update the affected
prerequisite tests to assert the corrected PowerShell argument shape and actual
command parameters rather than loosely matching installer flags.
In `@packages/run-spaceapp/tests/prerequisites.test.mjs`:
- Around line 49-52: Update the powershell.exe mock in the prerequisite test to
inspect the supplied argument values, including the expected executable data,
before marking installation successful or returning success; do not accept
--accept-license alone. Add a Windows-only assertion covering the final -Command
invocation and its populated arguments so missing data causes the regression
test to fail.
In `@scripts/verify-published-containers.mjs`:
- Around line 9-10: Update the release attestation configuration or the
verifier’s provenance predicate handling so both use the same expected SLSA
provenance version: configure the release image in the workflow to publish
version v1, or update the provenance predicate accepted by
verify-published-containers to match the emitted v0.2 URL. Preserve SBOM
verification and existing release validation behavior.
---
Nitpick comments:
In `@apps/api/tests/route-rate-limits.test.ts`:
- Around line 49-67: Replace or supplement the source-text offset checks in the
test named “registers the global limiter before request authentication and
lifecycle hooks” with an executable integration assertion using the Fastify app
instance and request path. Verify at runtime that the global rate limiter is
registered before authentication and lifecycle hooks execute, while optionally
retaining the existing structural check as a lightweight guard.
In `@scripts/verify-published-containers.mjs`:
- Around line 60-67: Update inspectRaw’s nonzero-status error path to include
result.stderr in the thrown error alongside the image identifier, while
preserving the existing JSON parse error handling.
- Around line 25-39: Update the attestation processing around the current find
call to inspect every attestation-manifest whose vnd.docker.reference.digest
matches platformManifest.digest, rather than stopping at the first match.
Aggregate the predicate types from all matching manifests into the existing
predicates Set, while preserving the current filtering of valid layer
annotations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 938a52fc-c24b-4be9-8609-fcdf3033baca
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (27)
.github/workflows/ci.yml.github/workflows/containers.yml.github/workflows/platform-matrix.yml.github/workflows/release.yml.github/workflows/security.ymlCHANGELOG.mdREADME.mdapps/api/src/browser-sessions.tsapps/api/tests/browser-capture-security.test.tsapps/api/tests/route-rate-limits.test.tsapps/web/package.jsonapps/web/server-routing.mjsapps/web/server.mjsapps/web/src/App.tsxapps/web/tests/server-routing.test.mjsdocs/operations.mddocs/public-release.mdpackage.jsonpackages/run-spaceapp/package.jsonpackages/run-spaceapp/src/cli.mjspackages/run-spaceapp/src/prerequisites.mjspackages/run-spaceapp/tests/cli.test.mjspackages/run-spaceapp/tests/prerequisites.test.mjsscripts/tests/public-package-metadata.test.mjsscripts/tests/published-container-verification.test.mjsscripts/verify-published-containers.mjstsconfig.base.json
💤 Files with no reviewable changes (1)
- tsconfig.base.json
Fixes compact multi-platform OCI attestation verification, removes CodeQL injection paths, proves API rate limiting, updates seven compatible Dependabot dependencies with immutable SHA pins, and prepares release 0.1.5. TypeScript stays on 5.9.3 because 7.0.2 removes the stable compiler API required by the route security regression test. Verified with the full public test suite, typechecks, build, zero-vulnerability npm audit, export verification, release readiness, registry preflight, real GHCR attestation verification, and npm package dry run.
Summary by CodeRabbit
New Features
/appredirects.Bug Fixes
Updates