refactor: files related to AWS deployment#374
Open
cherriechang wants to merge 75 commits intomainfrom
Open
Conversation
…ed builtFrontEnd var in deployFrontEnd()
…d against null distributionList.Items (line 671);
…ss hanging operations with timeout;
This commit adds comprehensive Auth0 authentication support to the basic site template, enabling OAuth2/OIDC authentication for Pushkin users. The implementation is backward-compatible and only activates when useAuth is enabled in pushkin.yaml.
Key Changes:
- Added Auth0 authentication components (Login, Logout, Profile)
- Added AuthSync component to synchronize Auth0 state with Redux
- Enhanced Redux infrastructure to handle both Auth0 and session-based auth
- Updated Header component with conditional auth buttons and 'My account' link
- Added conditional Auth0Provider wrapper in index.js
- Created config.js to expose authDomain and authClientID from pushkin.yaml
- Added @auth0/auth0-react dependency to package.json
- Added /profile route to App.js
Features:
- Dual authentication modes: Auth0 (when configured) and session-based (fallback)
- Conditional rendering based on useAuth config setting
- Seamless integration with existing Redux state management
- User profile management via Auth0 dashboard
- Automatic token refresh and localStorage caching
Configuration:
Sites can enable Auth0 by setting in pushkin.yaml:
addons:
useAuth: true
authDomain: your-domain.auth0.com
authClientID: your-client-id
All changes are backward-compatible. Sites without Auth0 configuration will continue using session-based authentication.
- Header's useEffect now skips dispatching getUser() when Auth0 is enabled - Auth0 users are managed by AuthSync component via SET_AUTH0_USER action - This prevents session-based user IDs from replacing Auth0 user IDs in Redux 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add heartbeat: 30 configuration to amqp.connect() in pushkin-worker - Upgrade RabbitMQ from 3.6 to 3.12 in docker-compose template - Add RABBITMQ_HEARTBEAT environment variable This fixes the "Heartbeat timeout" error that was preventing experiment workers from completing database operations. The 30-second heartbeat interval ensures the connection stays alive during long-running tasks.
…ections - Fix "Cannot read properties of undefined" error by checking if worker service exists - Use 'test_db' and 'test_transaction_db' Docker service names instead of localhost - Use internal port 5432 instead of host-mapped port for TRANS_PORT - Fixes ECONNREFUSED errors when workers try to connect to databases This ensures workers can communicate with databases via Docker networking.
session_id persistence in users database
…TaskCreator() Problem: When processing multiple workers (e.g., ew_test_worker, ew_test_2_worker): 1. First iteration created a reference to the template object 2. Modified the Docker image property (which mutated the shared template) 3. Second iteration reused the same mutated template object 4. Result: All workers ended up with the last processed worker's Docker image This caused deployment failures where: - ew_test_worker service used ew_test_2_worker:latest image (wrong!) - Workers consumed from incorrect RabbitMQ queues - Experiments failed to respond to API requests
…(cloudFrontClient handles auth)
… bucket and/or cloud ARNs do not resolve yet)
…rather than url (localhost, just for host machine access)
…ies: try/catch only catches synchronous errors, and DefaultHandler constructor cannot be async, so no await here)
…L:true env flag on AWS deployment, set ssl config in exp worker connection settings based on flag
…e info - prevent this with structuredClone
…ized clientFactory module - Handles both string and object profile formats (backward compatible) - Single configuration point for all AWS clients - Supports 14 AWS service clients (RDS, S3, ECS, etc.) Added unit tests to verify functionality
…potent), only create new when it's missing
…ections - Fix "Cannot read properties of undefined" error by checking if worker service exists - Use 'test_db' and 'test_transaction_db' Docker service names instead of localhost - Use internal port 5432 instead of host-mapped port for TRANS_PORT - Fixes ECONNREFUSED errors when workers try to connect to databases This ensures workers can communicate with databases via Docker networking.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
| const workerConfig = expConfig.worker; | ||
| const workerName = `${exp}_worker`.toLowerCase(); //Docker names must all be lower case | ||
| const workerLoc = path.join(expDir, workerConfig.location).replace(/ /g, "\\ "); //handle spaces in path |
Check failure
Code scanning / CodeQL
Incomplete string escaping or encoding High
| let workerBuild; | ||
| try { | ||
| workerBuild = exec( | ||
| `docker buildx build --platform linux/amd64 ${workerLoc} -t ${workerName} --load`, |
Check warning
Code scanning / CodeQL
Shell command built from environment values Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 11 days ago
General approach: Stop building shell command strings that embed environment-derived values and instead pass those values as separate arguments to exec/execSync variants that do not invoke a shell (or at least ensure proper argument escaping). In Node, this typically means using child_process.execFile / execFileSync or a similar wrapper that accepts a command and an array of arguments. If ../constants.js’s exec helper is a thin wrapper around child_process.exec, we cannot change its implementation here, but for locations we control directly (execSync import) we can switch to a safer API and avoid string interpolation altogether.
Concrete best fix in this snippet:
- For the vulnerable worker build in
rebuildWorker, stop constructing one long string with${workerLoc}and${workerName}. Instead, import a non-shell-spawning API (e.g.,execFilefromchild_process) and calldockerwith an explicit argument array:["buildx", "build", "--platform", "linux/amd64", workerLoc, "-t", workerName, "--load"]. This will correctly handle any special characters inworkerLocandworkerNamewithout relying on shell parsing and makes the current.replace(/ /g, "\\ ")unnecessary. - Remove the manual space escaping on
workerLoc, since it is no longer needed when passing it as a direct argument. - Keep functionality unchanged otherwise: same working directory, same Docker command semantics, same return behavior.
We only need to edit packages/pushkin-cli/src/commands/aws/services/docker.js in the shown region:
- Update the import from
child_processto includeexecFile. - Modify
workerLocconstruction to drop the.replace(/ /g, "\\ "). - Replace the
execcall that builds a single string inrebuildWorkerwith anexecFilecall using an argument array and a callback that resolves/rejects aPromiseso the function still returns something awaitable.
packages/pushkin-cli/src/commands/aws/index.js does not need changes; it only reads directory names and passes them to rebuildWorker, but no shell command is built there.
Suggested changeset
1
packages/pushkin-cli/src/commands/aws/services/docker.js
| @@ -1,4 +1,4 @@ | ||
| import { execSync } from "child_process"; | ||
| import { execSync, execFile } from "child_process"; | ||
| import fs from "graceful-fs"; | ||
| import path from "path"; | ||
| import jsYaml from "js-yaml"; | ||
| @@ -115,13 +115,32 @@ | ||
| } | ||
| const workerConfig = expConfig.worker; | ||
| const workerName = `${exp}_worker`.toLowerCase(); //Docker names must all be lower case | ||
| const workerLoc = path.join(expDir, workerConfig.location).replace(/ /g, "\\ "); //handle spaces in path | ||
| const workerLoc = path.join(expDir, workerConfig.location); | ||
|
|
||
| let workerBuild; | ||
| try { | ||
| workerBuild = exec( | ||
| `docker buildx build --platform linux/amd64 ${workerLoc} -t ${workerName} --load`, | ||
| ); | ||
| workerBuild = await new Promise((resolve, reject) => { | ||
| execFile( | ||
| "docker", | ||
| [ | ||
| "buildx", | ||
| "build", | ||
| "--platform", | ||
| "linux/amd64", | ||
| workerLoc, | ||
| "-t", | ||
| workerName, | ||
| "--load", | ||
| ], | ||
| (error, stdout, stderr) => { | ||
| if (error) { | ||
| reject(error); | ||
| return; | ||
| } | ||
| resolve({ stdout, stderr }); | ||
| }, | ||
| ); | ||
| }); | ||
| } catch (e) { | ||
| console.error(`Problem building worker for ${exp}`); | ||
| throw e; |
Copilot is powered by AI and may make mistakes. Always verify output.
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
4 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.
This should be more easily merged into main after fixAWS branch is merged.