spec(projects): define Project preview URL allowlist#1677
Draft
jsell-rh wants to merge 1 commit into
Draft
Conversation
✅ Deploy Preview for cheerful-kitten-f556a0 canceled.
|
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR specifies ProjectSettings, a per-Project singleton resource for managing preview URL trust policy with conditional Ambient token relay. The changes span the data model (Kind definition, storage, GitOps sync), API/CLI interface (project-scoped routes and apply semantics), RBAC authorization (permissions and scope resolution), and UI integration (Live Preview hardening and Settings Preview tab). ChangesProjectSettings: Preview Trust Configuration
Possibly Related PRs
🚥 Pre-merge checks | ✅ 8✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@specs/ambient-ui/ambient-ui.spec.md`:
- Around line 834-835: Add a session-scoped BFF endpoint that enforces Session
annotation-based preview URL validation: implement a handler (e.g.,
handlePreviewRequest) for the route pattern matching
/api/projects/{projectId}/sessions/{sessionId}/preview that calls a server-side
Session fetch (e.g., fetchSessionById(sessionId)), reads the
ambient-code.io/ui/preview-url annotation, and only accepts the incoming url
query when it equals that annotation or is a validated redirect derived from it
(extract/validate via a helper like validatePreviewUrl). Wire all proxied
requests and injected-link navigations through a proxyRequest flow that
revalidates the target against validatePreviewUrl and recomputes token relay per
target host (e.g., recomputeTokenRelay) before forwarding; return a 4xx when
validation fails.
In `@specs/api/ambient-model.spec.md`:
- Around line 478-487: Clarify and lock down the final ProjectSettings storage
schema and transition plan: explicitly state that ProjectSettings will have
separate persisted columns name (text, unique, not null), project_id (text,
unique, not null, foreign key -> projects.id), and spec (jsonb) while legacy
columns group_access and repositories may remain temporarily for transition but
are slated for removal; require rows be backfilled with name =
'<project-name>-settings' and spec.projectRef.name = <project-name> on
migration, and enforce immutability of project_id and spec.projectRef.name after
creation (describe the enforcement mechanism); update references to
ProjectSettings/ProjectId and any migration steps to indicate whether project_id
will be permanently denormalized or removed after dual-write, and include the
exact SQL constraints (unique non-deleted name and project_id, FK, not-null) to
be applied.
- Line 773: The ProjectSettings doc currently states metadata.name is immutable
after creation (ProjectSettings, metadata.name, spec.projectRef.name,
/projects/{id}/settings, acpctl apply) but does not warn that arbitrary names
will permanently lock future applies; either enforce the recommended convention
at creation or explicitly document the footgun—update the ProjectSettings
section to require/validate metadata.name follows the "{project}-settings"
pattern on create (reject otherwise) OR add a clear note explaining that
metadata.name is immutable, that first apply with a nonconforming name will
permanently lock that name (causing future acpctl apply to 409 unless deleted),
and provide the supported remediation (delete-and-recreate) so users are aware.
In `@specs/security/rbac-enforcement.spec.md`:
- Around line 146-151: Update the spec for the compatibility POST
/project_settings route to require a 400 Bad Request response when the payload
contains mismatched project references (e.g., project_id vs spec.projectRef.name
disagree), and specify that the error body MUST identify the conflicting fields
and their provided values (e.g., { "error": "project_reference_mismatch",
"fields": { "project_id": "proj-1", "spec.projectRef.name": "proj-2" } }) so
clients can correct the payload; reference the POST /project_settings
compatibility route and the fields project_id and spec.projectRef.name when
adding this requirement.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e4469263-d892-4263-bc73-2d8d0eeb0070
📒 Files selected for processing (3)
specs/ambient-ui/ambient-ui.spec.mdspecs/api/ambient-model.spec.mdspecs/security/rbac-enforcement.spec.md
jsell-rh
force-pushed
the
jsell/project-preview-allowlist
branch
from
2a0efcb to
59de1c6
Compare
jsell-rh
changed the title
jsell-rh
force-pushed
the
jsell/project-preview-allowlist
branch
from
59de1c6 to
ae2f36d
Compare
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
1 participant
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
spec.preview.allowedHosts.metadata,spec.description,spec.prompt, andspec.preview.Review Notes
Validation
git diff --check -- specs/api/ambient-model.spec.md specs/ambient-ui/ambient-ui.spec.md specs/security/rbac-enforcement.spec.mdrgcheck for stale ProjectSettings/project_settings references in touched specsgit commit --amendpre-commit hook: passed