Skip to content

fix: distinct RAND32 secrets, literal env writes, pinned kustomize (#547, #556)#561

Open
Flegma wants to merge 2 commits into
mainfrom
audit/panel-hardening
Open

fix: distinct RAND32 secrets, literal env writes, pinned kustomize (#547, #556)#561
Flegma wants to merge 2 commits into
mainfrom
audit/panel-hardening

Conversation

@Flegma

@Flegma Flegma commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

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.sh generated one random value per file and applied it with a global sed, 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.sh interpolated the value into sed, so a secret with sed metacharacters (| delimiter, &, trailing \) was corrupted or silently dropped (a Steam password p@ss|word broke the substitution). Now removes any existing line and appends with printf, writing the value verbatim. Verified with a metacharacter-laden value.

Pin kustomize installer (#556)
setup_kustomize.sh piped install_kustomize.sh from the moving master branch straight into bash. 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 -n clean on all three scripts.
  • Functional tests (in a scratch dir) for the two #547 behaviors, both pass (see commit).

Deferred from this PR (documented on the issues)

  • [Infrastructure] mediamtx grants anonymous control-API + publish, bypassing stream-security auth #540 (mediamtx anonymous publish/api): the fix (restrict 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.
  • [Infrastructure] Tailscale auth key and OAuth client secret exposed via process arguments #560 (secrets in process args): the OAuth client secret (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 across tailscale-api.sh and game-node-server-setup.sh, and the tailscale 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).

Flegma added 2 commits July 10, 2026 22:04
…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.
@Flegma

Flegma commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Independent code review run on the diff. It caught that update_env_var's grep -v ... && mv skipped the mv when every line matched the key (a single-key file), leaving a duplicate/stale line plus a leaked .tmp. Fixed in 3f81a1e by decoupling mv from grep's exit code. The RAND32 and kustomize changes reviewed clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Infrastructure] Secret-substitution scripts collide paired secrets and corrupt values with sed metacharacters

1 participant