fix: distinct RAND32 secrets, literal env writes, pinned kustomize (#547, #556)#561
Open
Flegma wants to merge 2 commits into
Open
fix: distinct RAND32 secrets, literal env writes, pinned kustomize (#547, #556)#561Flegma wants to merge 2 commits into
Flegma wants to merge 2 commits into
Conversation
…ly, pin kustomize - replace_rand32_in_env_files: generate a fresh random value per $(RAND32) occurrence instead of one per file. The global sed made paired secrets identical (APP_KEY == ENC_SECRET, S3_ACCESS_KEY == S3_SECRET), so learning the semi-public key leaked its partner. Rewritten line by line with bash replace-first so values are distinct and never interpreted as sed patterns (#547). - update_env_var: stop feeding the value through sed. A secret containing sed metacharacters ('|', '&', trailing '\') corrupted or silently dropped the value. Now removes any existing line and appends with printf, writing the value verbatim (#547). - setup_kustomize: pin the installer to an immutable release tag instead of the moving master branch, and download-then-run instead of piping the network straight into bash (#556).
Review follow-up: when the file's only lines matched the key, grep -v exited 1 and the &&-gated mv was skipped, leaving the stale line plus the appended one (duplicate key + leaked .tmp). Decouple mv from grep's exit code so it always applies the filtered result.
Contributor
Author
|
Independent code review run on the diff. It caught that |
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
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.
Self-contained shell/secret-handling fixes from the 2026-07 audit. Closes #547, #556.
Changes
Distinct value per RAND32 secret + literal env writes (#547)
replace_rand32_in_env_files.shgenerated one random value per file and applied it with a globalsed, so two$(RAND32)placeholders in the same file (e.g.APP_KEY/ENC_SECRET,S3_ACCESS_KEY/S3_SECRET) got the same value. Now rewritten line by line with bash replace-first so each occurrence gets its own value, and no secret is ever interpreted as a sed pattern. Verified: three placeholders in one file produce three distinct values, no leftover placeholders, non-placeholder lines untouched.update_env_var.shinterpolated the value intosed, so a secret with sed metacharacters (|delimiter,&, trailing\) was corrupted or silently dropped (a Steam passwordp@ss|wordbroke the substitution). Now removes any existing line and appends withprintf, writing the value verbatim. Verified with a metacharacter-laden value.Pin kustomize installer (#556)
setup_kustomize.shpipedinstall_kustomize.shfrom the movingmasterbranch straight intobash. Now pins the installer to an immutable release tag (kustomize/v5.5.0), downloads it to a temp file, then runs it with the version and target dir (the installer checksum-verifies the kustomize binary it fetches).Testing
bash -nclean on all three scripts.#547behaviors, both pass (see commit).Deferred from this PR (documented on the issues)
authInternalUsers, bind the control API to localhost, drop the api port from the Service) changes live-streaming behavior. Without a dev stack to confirm the legitimate publisher and any in-cluster API caller keep working, shipping it blind risks taking streaming down. Needs a test pass on the streaming topology.curl -d), the access token (curl -H Authorization), and the tailscale auth key (tailscale up --authkey=) are all passed in argv. A correct fix routes them through temp files / stdin consistently acrosstailscale-api.shandgame-node-server-setup.sh, and thetailscale up --authkey=file:form can't be verified against the installed tailscale version here. Left out to avoid a half-fix or breaking the node-join path (P3, one-time interactive setup).