fix: harden config loading to prevent eval of error output#17
Closed
fix: harden config loading to prevent eval of error output#17
Conversation
npm's reify step explicitly removes symlinks with the warning "Removing non-directory /usercontent/node_modules" and creates a real directory. This means the symlink approach for caching node_modules on the PVC never worked — packages always installed to ephemeral storage and were lost on pod restart. Replace the symlink strategy with copy-based caching: - After npm install, copy node_modules to /data/node_modules on PVC - On restart, if lockfile hash matches and cache exists, restore node_modules from PVC cache instead of running npm install - Keep .next/cache symlink as-is (next build follows symlinks correctly) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The entrypoint script was eval'ing the raw output of the config-to-env CLI command without validation. When the CLI returned error messages (e.g., "Authorization: Bearer <token>" on auth failures), eval tried to execute them as shell commands, producing cryptic errors like "Authorization: command not found" and loading 0 environment variables. Changes: - Filter CLI output to only eval valid export statements (grep ^export) - Log actionable error messages when config loading fails - Show the raw CLI output for debugging when failures occur - Distinguish between CLI failure (non-zero exit) and empty/invalid output Ref: Eyevinn/osaas-ai#192 (Issues 1 and 6) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
|
Closing this PR as redundant — the changes here are covered by more focused PRs:
The The node_modules symlink → copy-from-PVC rewrite is a larger behavioral change that should be tracked separately if needed. |
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
Hardens the
docker-entrypoint.shconfig loading to prevent executing error messages as shell commands.Problem
When the
config-to-envCLI command fails (e.g., expired auth token), its error output was passed directly toeval, producing cryptic errors likeAuthorization: command not foundand loading 0 environment variables. The app would start in a broken state with no config.Changes
evallines matching^export [A-Za-z_][A-Za-z0-9_]*=— ignore any error/debug outputDefense in depth
This is a safety net for the web-runner side. The root cause (missing auth header in
config-to-env) is being fixed in EyevinnOSC/client-ts#96.Ref: Eyevinn/osaas-ai#192 (Issues 1 and 6)
Test plan
evalof error output$?check catches CLI failures correctly🤖 Generated with Claude Code