-
Notifications
You must be signed in to change notification settings - Fork 42
Fixed Secrets sharing #154
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.
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
Fixed Secrets sharing #154
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughSummary by CodeRabbit
WalkthroughThe changes update AWS credential handling in S3 file listing logic, adjust how secrets are passed in runtime code, and relax base64 secret validation. Additionally, a dependency version is updated, a new dependency is added, and a route path is modified for graph template upserts. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API (state-manager/app/routes.py)
participant DB Model (graph_template_model.py)
Client->>API: PUT /graph/{graph_name} (upsert)
API->>DB Model: Validate and store secret (base64 decode)
DB Model-->>API: Success or error
API-->>Client: Response
sequenceDiagram
participant Runtime
participant Node
participant AWS S3
Runtime->>Node: Pass secrets dict (aws_access_key_id, aws_secret_access_key, aws_region)
Node->>AWS S3: Initialize boto3 client with explicit credentials
Node->>AWS S3: List S3 objects
AWS S3-->>Node: Return file list
Node-->>Runtime: Return results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
state-manager/app/models/db/graph_template_model.py (1)
1-1: Remove unused import causing pipeline failure.The
remodule is imported but no longer used after removing the regex validation logic.-import reexosphere-runtimes/cloud-storage-runtime/nodes/list_s3_files.py (1)
33-36: Add error handling for empty S3 responses.The code assumes
response['Contents']always exists, but S3 returns responses without this key when no objects match the criteria, which will cause aKeyError.- return [ - self.Outputs(key=data['Key']) - for data in response['Contents'] - ] + return [ + self.Outputs(key=data['Key']) + for data in response.get('Contents', []) + ]Additionally, the
files_onlyandrecursiveinput parameters are defined but not used in the implementation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
exosphere-runtimes/cloud-storage-runtime/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
exosphere-runtimes/cloud-storage-runtime/nodes/list_s3_files.py(1 hunks)exosphere-runtimes/cloud-storage-runtime/pyproject.toml(1 hunks)python-sdk/exospherehost/_version.py(1 hunks)python-sdk/exospherehost/runtime.py(1 hunks)state-manager/app/models/db/graph_template_model.py(1 hunks)state-manager/app/routes.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: the exospherehost project is currently in beta phase, so breaking changes in apis are acceptable and...
Learnt from: NiveditJain
PR: exospherehost/exospherehost#131
File: state-manager/app/models/executed_models.py:0-0
Timestamp: 2025-08-03T04:26:00.148Z
Learning: The exospherehost project is currently in beta phase, so breaking changes in APIs are acceptable and don't require versioning or migration strategies.
Applied to files:
python-sdk/exospherehost/_version.pyexosphere-runtimes/cloud-storage-runtime/pyproject.toml
📚 Learning: the exospherehost project requires python versions > 3.12 for the ci workflow, meaning python 3.13 o...
Learnt from: NiveditJain
PR: exospherehost/exospherehost#130
File: .github/workflows/ci.yml:22-22
Timestamp: 2025-08-02T12:43:35.075Z
Learning: The exospherehost project requires Python versions > 3.12 for the CI workflow, meaning Python 3.13 or higher should be used despite potential stability concerns with pre-release versions.
Applied to files:
python-sdk/exospherehost/_version.pyexosphere-runtimes/cloud-storage-runtime/pyproject.toml
🪛 GitHub Actions: Ruff check on changed files only
state-manager/app/models/db/graph_template_model.py
[error] 1-1: Ruff: re imported but unused (F401). Remove unused import: re. 1 fixable error with the --fix option.
🔇 Additional comments (6)
python-sdk/exospherehost/_version.py (1)
1-1: LGTM! Version bump aligns with dependency management.The version increment from "0.0.7b3" to "0.0.7b4" is consistent with the runtime dependency pinning to "0.0.7b3" in the pyproject.toml file, following proper versioning practices for the beta phase.
exosphere-runtimes/cloud-storage-runtime/pyproject.toml (1)
9-9: LGTM! Appropriate dependency pinning.Adding the pinned dependency on
exospherehost==0.0.7b3ensures the runtime uses a stable version of the SDK while development continues on the next version.state-manager/app/routes.py (1)
118-118: No internal references to/graph-templates– ready to mergeA quick ripgrep search (
rg -i "graph-templates") returned no hits elsewhere in the repo. The only change lives in:
- state-manager/app/routes.py (line 118):
"/graph/{graph_name}",Since we’re in beta, this breaking change is acceptable. Remember to update any external clients to call
/graph/{graph_name}.python-sdk/exospherehost/runtime.py (1)
309-309: Secrets payload extraction confirmed
Verified that theget_secretsendpoint returns aSecretsResponseModelwith a top-levelsecretsfield (state-manager/app/models/secrets_response.py), so usingsecrets["secrets"]to initializenode.Secretscorrectly unwraps the nested dict. No further changes needed.state-manager/app/models/db/graph_template_model.py (1)
45-58: LGTM! Simplified validation logic by relying on base64 decoding.Removing the explicit regex validation and letting the
base64.urlsafe_b64decode()call handle invalid characters is a good simplification. The base64 decoding will naturally fail if invalid characters are present, making the regex check redundant.exosphere-runtimes/cloud-storage-runtime/nodes/list_s3_files.py (1)
18-21: LGTM! Improved security through explicit credential handling.The change from implicit environment variable usage to explicit AWS credential fields enhances security, traceability, and testability. All required fields for S3 authentication are properly defined.
No description provided.