Use daydream API to check if plugin is allowed#646
Merged
Conversation
Signed-off-by: Max Holland <[email protected]>
📝 WalkthroughWalkthroughReplaced local environment whitelist checks with an API-driven plugin allowlist: added Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Normalize as normalize_plugin_url()
participant Validator as is_plugin_allowed()
participant API as Daydream API
participant Handler as Response Handler
App->>Normalize: plugin_url
Normalize-->>App: normalized_url
App->>Validator: await is_plugin_allowed(normalized_url)
Validator->>API: GET /allowed-plugins?page=1
loop while more pages
API-->>Validator: plugins batch (paginated)
Validator->>API: GET next page
end
Validator-->>App: True / False or raise error
alt True
App->>Handler: proceed with plugin
else False
App->>Handler: return 403 Forbidden
else Error
App->>Handler: log error and handle gracefully
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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: 2
🧹 Nitpick comments (1)
src/scope/cloud/fal_app.py (1)
845-867: Advance pagination from the response, not the request.Line 867 always adds the requested
limit. If the/v1/pluginsendpoint returns fewer items than requested (common on the final page or due to internal API limits), this skips entries and can incorrectly reject an allowed plugin. Advanceoffsetby the actual number of items returned instead.🔄 Proposed fix
while True: resp = await client.get( base_url, params={ "remoteOnly": "true", "limit": limit, "offset": offset, }, timeout=10.0, ) resp.raise_for_status() data = resp.json() - for plugin in data.get("plugins", []): + plugins = data.get("plugins", []) + for plugin in plugins: plugin_url = plugin.get("repositoryUrl", "") if ( plugin_url and normalized_package == normalize_plugin_url(plugin_url) ): return True if not data.get("hasMore", False): break - offset += limit + offset += len(plugins)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scope/cloud/fal_app.py` around lines 845 - 867, The pagination increments offset by the requested limit which can skip items when the server returns fewer results; in the loop that calls client.get for "/v1/plugins" (using variables client, base_url, params limit/offset and checking plugins from data), change the offset advancement to use the actual number of items returned (e.g., len(data.get("plugins", [])) or a returned count field) instead of always adding limit, so references to normalized_package and normalize_plugin_url are checked against every returned plugin without skipping any.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/scope/cloud/fal_app.py`:
- Around line 31-32: The get_daydream_api_base function returns the raw
DAYDREAM_API_BASE env var which can include a trailing slash; update
get_daydream_api_base to read os.getenv("DAYDREAM_API_BASE",
"https://api.daydream.live") and strip any trailing slashes (e.g., using
rstrip('/')) before returning so callers (validation and plugin install paths)
never get a base URL ending with '/'.
- Around line 843-872: The code currently swallows all network/HTTP/JSON errors
and returns False from the allowlist lookup loop, making outages
indistinguishable from a denied plugin; change the exception path in the async
HTTP loop to return None instead of False (i.e., in the except Exception as e
block where you log with log_prefix() and base_url), and then update the caller
logic that inspects the lookup result (the check that compares
normalized_package via normalize_plugin_url and the subsequent boolean/403
decision) to detect None and respond with a 503/Service Unavailable and an
explanatory message rather than treating it as a plugin-not-found deny; ensure
you preserve existing True/False semantics for found/not-found but use None to
signal Daydream/API lookup failure.
---
Nitpick comments:
In `@src/scope/cloud/fal_app.py`:
- Around line 845-867: The pagination increments offset by the requested limit
which can skip items when the server returns fewer results; in the loop that
calls client.get for "/v1/plugins" (using variables client, base_url, params
limit/offset and checking plugins from data), change the offset advancement to
use the actual number of items returned (e.g., len(data.get("plugins", [])) or a
returned count field) instead of always adding limit, so references to
normalized_package and normalize_plugin_url are checked against every returned
plugin without skipping any.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b59022b4-de83-4094-9580-afd0ae47e82c
📒 Files selected for processing (1)
src/scope/cloud/fal_app.py
Contributor
🚀 fal.ai Preview Deployment
TestingConnect to this preview deployment by running this on your branch: 🧪 E2E tests will run automatically against this deployment. |
Contributor
✅ E2E Tests passed
Test ArtifactsCheck the workflow run for screenshots. |
Signed-off-by: Max Holland <[email protected]>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/scope/cloud/fal_app.py (1)
822-824:⚠️ Potential issue | 🟡 MinorShort-circuit invalid
packagevalues before the allowlist lookup.Lines 822-824 currently allow
requested_packageto be empty or non-string. Empty values force a full Daydream pagination walk before 403, and non-strings raise innormalize_plugin_url()and come back as a spurious 503. Return a client error — or let the Scope API validate first — before callingis_plugin_allowed().🩹 Proposed fix
if method == "POST" and normalized_path == "/api/v1/plugins": - requested_package = ( - body.get("package", "") if isinstance(body, dict) else "" - ) + requested_package = body.get("package") if isinstance(body, dict) else None + if not isinstance(requested_package, str) or not requested_package.strip(): + return { + "type": "api_response", + "request_id": request_id, + "status": 400, + "error": "Missing or invalid plugin package", + } + requested_package = requested_package.strip()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scope/cloud/fal_app.py` around lines 822 - 824, The code currently sets requested_package from body without validating type or emptiness, which lets empty or non-string values proceed into normalize_plugin_url() and is_plugin_allowed(); fix by short-circuiting: before calling is_plugin_allowed(), check that requested_package is an instance of str and not empty (or otherwise return a 400 client error / forward to Scope API validation), and reject non-string or empty values early to avoid triggering full pagination or normalize_plugin_url() errors; update the logic around the requested_package assignment and the subsequent call to is_plugin_allowed() to perform this validation first.
♻️ Duplicate comments (1)
src/scope/cloud/fal_app.py (1)
31-32:⚠️ Potential issue | 🟡 MinorStrip trailing slashes in
get_daydream_api_base().Line 32 still returns the raw env var, so
DAYDREAM_API_BASE=https://api.daydream.live/yields//v1/users/...and//v1/pluginsin the two current call sites.🩹 Proposed fix
def get_daydream_api_base() -> str: - return os.getenv("DAYDREAM_API_BASE", "https://api.daydream.live") + return os.getenv("DAYDREAM_API_BASE", "https://api.daydream.live").rstrip("/")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scope/cloud/fal_app.py` around lines 31 - 32, get_daydream_api_base currently returns the raw DAYDREAM_API_BASE which can include a trailing slash causing double-slash URLs; update get_daydream_api_base to strip any trailing slashes from the env value (e.g., use rstrip('/') on os.getenv(...) result or on the resulting string) and return that normalized base (ensure the default "https://api.daydream.live" is treated the same) so callers building paths don't produce double slashes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/scope/cloud/fal_app.py`:
- Around line 822-824: The code currently sets requested_package from body
without validating type or emptiness, which lets empty or non-string values
proceed into normalize_plugin_url() and is_plugin_allowed(); fix by
short-circuiting: before calling is_plugin_allowed(), check that
requested_package is an instance of str and not empty (or otherwise return a 400
client error / forward to Scope API validation), and reject non-string or empty
values early to avoid triggering full pagination or normalize_plugin_url()
errors; update the logic around the requested_package assignment and the
subsequent call to is_plugin_allowed() to perform this validation first.
---
Duplicate comments:
In `@src/scope/cloud/fal_app.py`:
- Around line 31-32: get_daydream_api_base currently returns the raw
DAYDREAM_API_BASE which can include a trailing slash causing double-slash URLs;
update get_daydream_api_base to strip any trailing slashes from the env value
(e.g., use rstrip('/') on os.getenv(...) result or on the resulting string) and
return that normalized base (ensure the default "https://api.daydream.live" is
treated the same) so callers building paths don't produce double slashes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 73524538-a504-4946-aed4-82027247bf7e
📒 Files selected for processing (1)
src/scope/cloud/fal_app.py
emranemran
approved these changes
Mar 11, 2026
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
2 participants
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 by CodeRabbit
We can approve/revoke plugins in the daydream admin pages now and this will be automatically reflected in the front end as well as this back end check.
Screenshot of admin page: