Implement Global Variables & Secrets Manager - #9
Conversation
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds persistent globals and secrets with a management modal, runtime template interpolation, generated Python and JavaScript support, page integration, and tests covering validation, HTTP execution, and code generation. ChangesGlobals and Secrets
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73bfe2184e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| globals, | ||
| secrets, |
There was a problem hiding this comment.
When a user edits only globals/secrets in the new manager and then clicks Run, this memoized callback is not recreated because the dependency array still omits globals and secrets, so these fields keep the previous values (often empty) even though the UI/code preview updated. Flows using {{global.*}} or {{secret.*}} therefore execute with stale or blank substitutions until some unrelated dependency changes.
Useful? React with 👍 / 👎.
| lines.push(`import os`); | ||
| lines.push(`SECRETS = {`); | ||
| secrets.forEach((s) => { | ||
| lines.push(` ${pyStr(s.key)}: os.environ.get(${pyStr(s.key)}, ${pyStr(s.value)}),`); |
There was a problem hiding this comment.
When a user has a real secret configured and opens or copies the generated Python, this writes the secret value into the source as the environment-variable fallback. That makes the secrets manager silently leak credentials into shared snippets or committed generated files; the generated runtime should reference the environment variable without embedding the browser-stored secret value.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/flow/runFlow.ts (1)
266-273: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winInterpolated secret leaks into run logs. The
toolnode returnsargsafter interpolation, so any{{secret.*}}value becomes part ofstate.last_outputand is captured instateSnapshot, which is rendered verbatim in the run-log panel (Index.tsx). That defeats the masking applied everywhere else for secrets.Consider redacting known secret values from node output/snapshots before logging (e.g. replace secret values with
••••in the emittedargs/snapshot).🤖 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 `@frontend/src/flow/runFlow.ts` around lines 266 - 273, Redact secret values from the interpolated args returned by the “tool” case in runFlow before they enter state.last_output or stateSnapshot. Reuse the existing secret values from opts.secrets, replacing each known value with the established masked placeholder while preserving non-secret interpolation and the simulated tool response.
🤖 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 `@frontend/src/flow/codegen.ts`:
- Around line 123-127: Remove plaintext secret values from both generated-code
paths: in frontend/src/flow/codegen.ts lines 123-127, update the Python
os.environ.get generation; in frontend/src/flow/codegen.ts lines 482-486, update
the JavaScript process.env fallback. Use an empty or omitted default so
generated flow.py and flow.js rely only on environment-provided secrets, then
update the corresponding assertions in globalsAndSecrets.test.ts.
- Around line 622-624: Update the generated http node assignments for url,
headersStr, and bodyStr to embed their config values using JSON.stringify,
matching the llm, tool, subagent, and human node generation paths. Preserve the
existing fallback to empty strings while ensuring backticks and interpolation
sequences remain literal in generated JavaScript.
In `@frontend/src/pages/Index.tsx`:
- Around line 587-588: Update the dependency array of the runFlowAction
useCallback to include globals and secrets, ensuring the callback uses the
latest manager edits while preserving its existing dependencies.
---
Outside diff comments:
In `@frontend/src/flow/runFlow.ts`:
- Around line 266-273: Redact secret values from the interpolated args returned
by the “tool” case in runFlow before they enter state.last_output or
stateSnapshot. Reuse the existing secret values from opts.secrets, replacing
each known value with the established masked placeholder while preserving
non-secret interpolation and the simulated tool response.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e24d5102-611a-4b28-b56b-899e968dcb9f
⛔ Files ignored due to path filters (1)
start_output.logis excluded by!**/*.log
📒 Files selected for processing (6)
frontend/src/flow/GlobalsManager.tsxfrontend/src/flow/codegen.tsfrontend/src/flow/globals.tsfrontend/src/flow/runFlow.tsfrontend/src/pages/Index.tsxfrontend/src/test/globalsAndSecrets.test.ts
| lines.push(`SECRETS = {`); | ||
| secrets.forEach((s) => { | ||
| lines.push(` ${pyStr(s.key)}: os.environ.get(${pyStr(s.key)}, ${pyStr(s.value)}),`); | ||
| }); | ||
| lines.push(`}`); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Secrets embedded as plaintext fallbacks in generated code. Both generators inline the actual secret value as the env fallback default, so a copied/downloaded flow.py/flow.js carries secrets in cleartext — contradicting the module's "Sensitive keys/values never leave the browser" contract in globals.ts. Consider emitting an empty/placeholder default (e.g. rely on env only) and update the corresponding assertions in globalsAndSecrets.test.ts.
frontend/src/flow/codegen.ts#L123-L127: drop thepyStr(s.value)plaintext default fromos.environ.get(...)(e.g. use""or omit the default).frontend/src/flow/codegen.ts#L482-L486: drop theJSON.stringify(s.value)plaintext fallback fromprocess.env.<key> ?? ....
📍 Affects 1 file
frontend/src/flow/codegen.ts#L123-L127(this comment)frontend/src/flow/codegen.ts#L482-L486
🤖 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 `@frontend/src/flow/codegen.ts` around lines 123 - 127, Remove plaintext secret
values from both generated-code paths: in frontend/src/flow/codegen.ts lines
123-127, update the Python os.environ.get generation; in
frontend/src/flow/codegen.ts lines 482-486, update the JavaScript process.env
fallback. Use an empty or omitted default so generated flow.py and flow.js rely
only on environment-provided secrets, then update the corresponding assertions
in globalsAndSecrets.test.ts.
| `let url = \`${c.url || ""}\`;`, | ||
| `let headersStr = \`${headersJson}\`;`, | ||
| `let bodyStr = \`${c.body || ""}\`;`, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Unsafe/inconsistent template embedding in generated JS http node. url, headersStr, and bodyStr are emitted as backtick template literals with the raw config inlined. Every other JS node (llm, tool, subagent, human) uses JSON.stringify(...). A backtick or ${...} in the URL/headers/body will break the generated file or be evaluated at generated-code parse time (injection).
🐛 Use JSON.stringify for consistency and safety
- `let url = \`${c.url || ""}\`;`,
- `let headersStr = \`${headersJson}\`;`,
- `let bodyStr = \`${c.body || ""}\`;`,
+ `let url = ${JSON.stringify(c.url || "")};`,
+ `let headersStr = ${JSON.stringify(headersJson)};`,
+ `let bodyStr = ${JSON.stringify(c.body || "")};`,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| `let url = \`${c.url || ""}\`;`, | |
| `let headersStr = \`${headersJson}\`;`, | |
| `let bodyStr = \`${c.body || ""}\`;`, | |
| `let url = ${JSON.stringify(c.url || "")};`, | |
| `let headersStr = ${JSON.stringify(headersJson)};`, | |
| `let bodyStr = ${JSON.stringify(c.body || "")};`, |
🤖 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 `@frontend/src/flow/codegen.ts` around lines 622 - 624, Update the generated
http node assignments for url, headersStr, and bodyStr to embed their config
values using JSON.stringify, matching the llm, tool, subagent, and human node
generation paths. Preserve the existing fallback to empty strings while ensuring
backticks and interpolation sequences remain literal in generated JavaScript.
| globals, | ||
| secrets, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Stale closure: add globals/secrets to runFlowAction deps. These new references are read inside the useCallback, but its dependency array (Line 637) omits globals and secrets. The memoized callback will keep capturing the values from the render when it was last recreated, so edits made in the manager won't be reflected on the next run.
🐛 Add the missing dependencies (Line 637)
}, [nodes, edges, gateways, gatewayInvalid, gatewayIssues, visualSpeed, initialStateStr, pendingApproval, globals, secrets]);🤖 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 `@frontend/src/pages/Index.tsx` around lines 587 - 588, Update the dependency
array of the runFlowAction useCallback to include globals and secrets, ensuring
the callback uses the latest manager edits while preserving its existing
dependencies.
1 participant
Implemented a fully client-side Environment & State Constants manager for global variables and secrets. Support has been fully integrated into the visual canvas, run execution engine, and live code generators for Python and JavaScript. Also added extensive unit testing.
PR created automatically by Jules for task 16643472704020951248 started by @Jacobcdsmith
Summary by CodeRabbit
New Features
{{global.*}}and{{secret.*}}placeholders in flows.Tests