docs: document subprocess environment filtering and the passthrough setting - #308
Open
JeffreyDallas wants to merge 8 commits into
Open
docs: document subprocess environment filtering and the passthrough setting#308JeffreyDallas wants to merge 8 commits into
JeffreyDallas wants to merge 8 commits into
Conversation
…etting Solo builds a per-command allowlist for the external tools it runs, so a variable set in the user's shell may not reach helm or kubectl. Nothing documented that, which is part of why hiero-ledger/solo#5895 (EKS IRSA broken by a missing variable) was hard to diagnose. Adds a page covering how to tell from solo.log that a variable was withheld, how to forward one with subprocess.additionalEnvironmentVariables, why that setting is config-file only, which names are refused unconditionally, and the current state of EKS/GKE/AKS support. Cross-links it from the environment variables page, which documents variables that configure Solo itself rather than ones passed on. Related to hiero-ledger/solo#5895 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Jeffrey Tang <jeffrey@swirldslabs.com>
JeffreyDallas
marked this pull request as draft
August 26, 2026 17:25
Review of hiero-ledger/solo#5897 noted the page promised users could search solo.log for any withheld variable, while the implementation bounded the listing. Documents both bounds explicitly - the identifier shape restriction and the 2000-name ceiling - and makes clear they affect only what is reported, never what is forwarded. Also records that AWS_ENDPOINT_URL, LD_ and DYLD_ are refused by prefix rather than exact name, so AWS_ENDPOINT_URL_STS cannot redirect the EKS credential exchange, and states where the config file lives and that it is optional. Related to hiero-ledger/solo#5895 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Jeffrey Tang <jeffrey@swirldslabs.com>
~/.solo/solo.yaml is a leftover from older Solo versions holding an unrelated flags: structure, and the solo:config:remove task in hiero-consensus-node's CITR Taskfile deletes it automatically - settings placed there would be silently lost. Names the correct file, states where it lives, and warns about the similarly named legacy file. Related to hiero-ledger/solo#5895 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Jeffrey Tang <jeffrey@swirldslabs.com>
…examples Review of solo-docs#308 noted the page claimed Solo restricts ~/.solo on creation, while users are told to create solo-config.yaml themselves - Solo never owned that file's permissions. Solo now validates the file instead of assuming: hiero-ledger/solo#5897 refuses to apply the settings, with an error, when the file or its directory is a symbolic link, is not owned by the current user, or is group/other writable. The page now describes that behaviour, shows how to keep both owner-only, and notes that only the symlink check applies on Windows, where there are no POSIX mode bits. Also adds PowerShell equivalents for the log search, the SOLO_HOME override and the permission commands, and documents that a present-but-unusable config file is a hard error rather than a silent fallback. Related to hiero-ledger/solo#5895 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Jeffrey Tang <jeffrey@swirldslabs.com>
Review noted the PowerShell example hardened only solo-config.yaml, while write access to the containing directory is enough to replace the file inside it. Both the Bash and PowerShell examples now secure the directory as well. Also describes what hiero-ledger/solo#5897 actually checks on Windows - the DACL is read with icacls and any write grant to a principal other than the user, SYSTEM, Administrators or CREATOR OWNER is refused, with inherit-only entries ignored - and records that the whole ancestor chain is validated and the file is read through a no-follow descriptor. Adds a warning that Windows behaviour has not been exercised on a real Windows machine, so the guide no longer presents it as verified. Related to hiero-ledger/solo#5895 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Jeffrey Tang <jeffrey@swirldslabs.com>
Review of hiero-ledger/solo#5897 noted the page promised more than the code delivers: it said every ancestor symlink or non-user-owned directory is rejected and that the file cannot be swapped. Neither was accurate. POSIX accepts root-owned and sticky ancestors by necessity, Windows stops before the volume root because C:\ legitimately carries broad write grants, and the no-follow descriptor protects only the final component - Node offers no component-by-component open, so a directory swap by someone who already has write access on the path is not defended against. The page now separates what is guaranteed (the file's contents, read through the validated descriptor) from what is best-effort (the directory checks, which catch a misconfigured or shared SOLO_HOME rather than an active attacker), and says so in one sentence at the end. Also strengthens the Windows warning from "unverified" to "not yet usable", since the guarantee there is weaker and the code has never run on Windows. Related to hiero-ledger/solo#5895 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Jeffrey Tang <jeffrey@swirldslabs.com>
Review of solo-docs#308 noted the descriptor guarantee read as universal while the Windows caveat further down correctly describes a check-then-open. Scopes the earlier statement to POSIX and points at that note, so the two agree. Related to hiero-ledger/solo#5895 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Jeffrey Tang <jeffrey@swirldslabs.com>
The snippet put variable names on the summary line. Solo actually writes a summary ending in "for that command:" and then one or more "withheld from '<command>':" lines carrying the names, so a reader comparing the example to their own log would not find a match. Replaces it with output copied from a real run and explains that searching for a variable name lands on the second line, which also identifies the command. Related to hiero-ledger/solo#5895 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Jeffrey Tang <jeffrey@swirldslabs.com>
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.
Description
Documents how Solo filters the environment it passes to the external commands it runs (
helm,kubectl,kind, container engines), which was previously undocumented anywhere.Companion to hiero-ledger/solo#5897, which fixes #5895 — EKS IRSA broke because
AWS_ROLE_ARNandAWS_WEB_IDENTITY_TOKEN_FILEwere not on Solo's allowlist. Part of why that needed a bug report is that nothing told users the filtering existed at all; the reporter had to replace thehelmbinary with a recorder to find it.New page, Subprocess Environment Filtering:
solo.logthat a variable was withheld, with the real two-line log shape and both Bash and PowerShell search commandssubprocess.additionalEnvironmentVariables, including the per-command syntax and where the file livesSOLO_*AWS_ENDPOINT_URL,LD_andDYLD_are refused by prefix, soAWS_ENDPOINT_URL_STScannot redirect the EKS credential exchangeAlso adds a short cross-reference from the existing Using Environment Variables page, which documents variables that configure Solo itself — a distinction not previously drawn, and the likely first place a confused user looks.
Notes for reviewers
Several statements in this page were corrected during review of the companion PR, and the current text is deliberately narrower than the first draft:
solo-config.yaml, notsolo.yaml.~/.solo/solo.yamlis a leftover from older Solo versions and is deleted automatically by hiero-consensus-node's CITR tooling, so settings placed there would be silently lost. The page warns about the similarly named file.openat, so a directory swap by someone who already has write access on the path is not defended against. The page now separates the guaranteed part from the best-effort part and says so in one sentence.for that command:followed bywithheld from '<command>':lines, so a reader comparing the example to their own log would not have found a match.SOLO_HOMEoverride and the permission commands —icacls, not achmodthat does not exist there — and secure the.solodirectory as well as the file, since directory write access alone permits replacing it.Related Issues
AWS_ROLE_ARNandAWS_WEB_IDENTITY_TOKEN_FILEare stripped from helm solo#5895Testing
remark-cli --frailpasses on both changed files. Rendering was not verified in a local Hugo build.The GKE/AKS section deliberately says those platforms are not covered rather than listing guessed variable names, matching the code: the companion PR speculatively added
AZURE_AUTHORITY_HOSTand review correctly identified it as an authority-redirect vector. If someone verifies the required set on a real cluster, the docs and the built-in allowlist should be updated together.