Skip to content

Display env vars with concealed value by default.#2440

Merged
fnando merged 6 commits intomainfrom
env-vars-hide-by-default
Mar 10, 2026
Merged

Display env vars with concealed value by default.#2440
fnando merged 6 commits intomainfrom
env-vars-hide-by-default

Conversation

@fnando
Copy link
Member

@fnando fnando commented Mar 10, 2026

What

$ STELLAR_SECRET_KEY=abc STELLAR_RPC_HEADERS=a=1 STELLAR_SIGN_WITH_KEY=b target/debug/stellar env
STELLAR_ACCOUNT=default           # use
STELLAR_NETWORK=local             # use
STELLAR_RPC_HEADERS=<concealed>   # env
STELLAR_SECRET_KEY=<concealed>    # env
STELLAR_SIGN_WITH_KEY=<concealed> # env

Also, getting any non-visible value will return an empty string.

Why

https://hackerone.com/reports/3596218

Known limitations

N/A

Copilot AI review requested due to automatic review settings March 10, 2026 17:50
@github-project-automation github-project-automation bot moved this to Backlog (Not Ready) in DevX Mar 10, 2026
@fnando fnando requested review from leighmcculloch and removed request for Copilot March 10, 2026 17:50
@fnando fnando self-assigned this Mar 10, 2026
@fnando fnando moved this from Backlog (Not Ready) to Needs Review in DevX Mar 10, 2026
@fnando fnando requested a review from a team March 10, 2026 17:50
@fnando fnando requested a review from Copilot March 10, 2026 19:05
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the stellar env command to avoid leaking sensitive environment variable values by default, printing <concealed> for non-visible variables and returning an empty string when a user queries a non-visible key directly.

Changes:

  • Expanded the supported env var list to explicitly include secret-ish vars (e.g., SECRET_KEY, SIGN_WITH_KEY) while controlling display via a visibility allowlist.
  • Updated stellar env output formatting to conceal non-visible values and suppress direct value output for non-visible keys.
  • Added/updated integration tests to ensure sensitive values (RPC headers, secret key, sign-with-key) are not displayed.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
cmd/soroban-cli/src/env_vars.rs Adds secret vars to the supported list and introduces is_visible allowlist logic.
cmd/soroban-cli/src/commands/env/mod.rs Applies is_visible to conceal values in list output and to suppress single-key secret lookups.
cmd/soroban-cli/src/cli.rs Simplifies env-var mapping by relying on the expanded unprefixed() list.
cmd/crates/soroban-test/tests/it/config.rs Adds tests to confirm concealed output and absence of sensitive substrings.

Copy link
Member

@leighmcculloch leighmcculloch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's not much information in the why as to why this change is being made. This seems like a pretty severe breaking change. And prevents meaningful use doesn't it? What does this protect against, anyone can simply run the env command and see all the variables. Is there a way to override to get the actual data?

@fnando
Copy link
Member Author

fnando commented Mar 10, 2026

A few things:

  • Secret values shouldn't be visible (we were completely omitting them, but rpc headers was an oversight). This makes sure STELLAR_SIGN_WITH_KEY, STELLAR_RPC_HEADERS and STELLAR_SECRET_KEY are not accessible.
  • It's very hard to debug the source if they're not even showing up on stellar env, so I decided to show them as <concealed>
  • Future work will add --no-masking (or --reveal-concealed), so they're added to stellar env

If we think this is too much, I can remove the "concealed" part for now, and make sure it's added during the protocol upgrade release. Unfortunately, there's no way we can make this backwards compatible.

@leighmcculloch
Copy link
Member

  • It's very hard to debug the source if they're not even showing up on stellar env, so I decided to show them as <concealed>

Maybe those lines should be comments, so prefixed with a # , that way if anyone takes the output and pushes it into a shell those lines don't overwrite the current envs if they are set as env vars.

@leighmcculloch
Copy link
Member

  • Future work will add --no-masking (or --reveal-concealed), so they're added to stellar env

--reveal-concealed sgtm and the common term 'concealed' makes it easier to connect mentally.

@fnando
Copy link
Member Author

fnando commented Mar 10, 2026

Maybe those lines should be comments, so prefixed with a #

great idea!

@fnando
Copy link
Member Author

fnando commented Mar 10, 2026

done:

$ STELLAR_SECRET_KEY=abc STELLAR_RPC_HEADERS=a=1 STELLAR_SIGN_WITH_KEY=b target/debug/stellar env
STELLAR_ACCOUNT=default             # use
STELLAR_NETWORK=local               # use
# STELLAR_RPC_HEADERS=<concealed>   # env
# STELLAR_SECRET_KEY=<concealed>    # env
# STELLAR_SIGN_WITH_KEY=<concealed> # env

@fnando fnando enabled auto-merge (squash) March 10, 2026 22:36
@fnando fnando merged commit 6b2e3ea into main Mar 10, 2026
179 of 193 checks passed
@fnando fnando deleted the env-vars-hide-by-default branch March 10, 2026 23:55
@github-project-automation github-project-automation bot moved this from Needs Review to Done in DevX Mar 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants