Skip to content

Stop echoing a mistyped secret given where a contract id is expected - #2795

Merged
fnando merged 6 commits into
mainfrom
conceal-mistyped-secret-in-contract-id
Oct 14, 2026
Merged

fnando merged 6 commits into
mainfrom
conceal-mistyped-secret-in-contract-id

Conversation

@fnando

@fnando fnando commented Oct 5, 2026

Copy link
Copy Markdown
Member

What

Commands sharing UnresolvedContract (--id/STELLAR_CONTRACT_ID) echoed the raw input in the contract not found (and, for a seed phrase, Invalid name) error, leaking a secret key or seed phrase pasted there. The error now conceals secret-shaped input (custom Debug too), while still naming a genuine alias typo so you can see which value was wrong. Same conditional naming extended to the address params from #2785.

The contract-id counterpart to #2763 and #2785, which didn't cover this path.

$ stellar contract fetch --id SBF5HLRREHMS36XZNTUSKZ6FTXDZGNXOHF4EXKUL5UCWZLPBX3NGJ4BX --network testnet
❌ error: contract not found

$ stellar contract fetch --id "illness spike retreat truth genius clock brain pass fit cave bargain xyzzy" --network testnet
❌ error: contract not found

$ stellar contract fetch --id nosuchalias --network testnet
❌ error: contract not found: nosuchalias

$ stellar token balance --id native --account SBF5HLRREHMS36XZNTUSKZ6FTXDZGNXOHF4EXKUL5UCWZLPBX3NGJ4BX
❌ error: invalid address or alias

$ stellar token balance --id native --account "illness spike retreat truth genius clock brain pass fit cave bargain xyzzy"
❌ error: invalid address or alias

$ stellar token balance --id native --account nosuchalias
❌ error: address alias 'nosuchalias' not found

Why

A secret is a bearer credential; a one-char typo still reveals effectively all of it. The new vector is STELLAR_CONTRACT_ID, echoed without ever being typed. Detection is shape-based because a mistyped secret no longer parses.

Known limitations

A 50+ char all-base32 alias starting with S, or a 12+ word alias, is treated as secret-shaped and not named.

Copilot AI lite review requested due to automatic review settings October 5, 2026 18:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Unresolved critical redaction and secret-detection issues, plus error handling that can hide unrelated failures.

Review effort: Lite
Findings: 3 High severity · 1 Medium severity

Open (4)
What changed in this PR

This PR prevents secret-shaped contract IDs and addresses from being echoed in errors or debug output while preserving useful alias diagnostics.

Changes:

  • Adds secret-shape detection and conditional redaction.
  • Updates contract and address resolution errors.
  • Adds unit and integration coverage.
File Description
cmd/​soroban-cli/​src/​config/​secret.rs Secret-shape detection and tests
cmd/​soroban-cli/​src/​config/​sc_address.rs Conditional address error concealment
cmd/​soroban-cli/​src/​config/​locator.rs Conditional contract error concealment
cmd/​soroban-cli/​src/​config/​alias.rs Contract resolution and debug protection
cmd/​crates/​soroban-test/​tests/​it/​config.rs CLI integration tests

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread cmd/soroban-cli/src/config/locator.rs Outdated
Comment thread cmd/soroban-cli/src/config/sc_address.rs
Comment thread cmd/soroban-cli/src/config/secret.rs Outdated
Comment thread cmd/soroban-cli/src/config/alias.rs Outdated
@fnando fnando self-assigned this Oct 5, 2026
Comment thread cmd/soroban-cli/src/config/sc_address.rs Dismissed
Copilot AI lite review requested due to automatic review settings October 5, 2026 19:08
@fnando
fnando force-pushed the conceal-mistyped-secret-in-contract-id branch from 9de4fbf to 41babaa Compare October 5, 2026 19:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Unresolved critical and moderate redaction gaps remain in alias errors, debug formatting, prefix mutations, and Unicode typo detection.

Review effort: Lite
Findings: 3 High severity

Open (3)
Resolved since last review (3)

Comment thread cmd/soroban-cli/src/config/alias.rs Outdated
Comment thread cmd/soroban-cli/src/config/secret.rs
Copilot AI lite review requested due to automatic review settings October 5, 2026 19:58
@fnando
fnando force-pushed the conceal-mistyped-secret-in-contract-id branch from 41babaa to e87b87b Compare October 5, 2026 19:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comment thread cmd/soroban-cli/src/config/locator.rs Outdated
Comment thread cmd/soroban-cli/src/config/secret.rs Outdated
Copilot AI lite review requested due to automatic review settings October 5, 2026 21:06
@fnando
fnando force-pushed the conceal-mistyped-secret-in-contract-id branch from e87b87b to 9cb45f5 Compare October 5, 2026 21:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

A critical 11-token seed-phrase detection gap and a moderate terminal-escape injection issue remain unresolved.

Review effort: Lite
Findings: 1 High severity

Open (1)
Resolved since last review (2)

Comment thread cmd/soroban-cli/src/config/secret.rs Outdated
Copilot AI lite review requested due to automatic review settings October 6, 2026 17:21
@fnando
fnando force-pushed the conceal-mistyped-secret-in-contract-id branch from 9cb45f5 to 9dc38a9 Compare October 6, 2026 17:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

One critical and two moderate secret-redaction issues remain unresolved.

Review effort: Lite
Findings: 1 High severity

Open (1)
Resolved since last review (1)

Comment thread cmd/soroban-cli/src/config/sc_address.rs
Copilot AI lite review requested due to automatic review settings October 6, 2026 18:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

Alias and debug output can still emit unescaped terminal control characters.

Review effort: Lite
Findings: None

Resolved since last review (1)

Copilot AI lite review requested due to automatic review settings October 6, 2026 20:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

No unresolved blocking issues were identified.

Review effort: Lite
Findings: None

@fnando
fnando requested review from a team and leighmcculloch October 6, 2026 20:19
Copilot AI lite review requested due to automatic review settings October 13, 2026 19:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

set_authorized.rs still calls the changed resolver with the old signature, so the workspace does not build.

1 open finding

🧠 Review effort: Lite

Comment thread cmd/soroban-cli/src/config/sc_address.rs
Copilot AI lite review requested due to automatic review settings October 13, 2026 20:43
@fnando
fnando enabled auto-merge (squash) October 13, 2026 20:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Fix the JSON-mode quiet-output handling in set_authorized.rs.

1 open finding
1 resolved since last review

🧠 Review effort: Lite

Comment thread cmd/soroban-cli/src/commands/token/set_authorized.rs Outdated
Copilot AI lite review requested due to automatic review settings October 13, 2026 23:01
@fnando
fnando force-pushed the conceal-mistyped-secret-in-contract-id branch from cd1dc27 to 1a6b037 Compare October 13, 2026 23:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Leading or trailing whitespace can bypass secret detection and leak the untrimmed secret in errors.

1 open finding
1 resolved since last review

🧠 Review effort: Lite

Comment thread cmd/soroban-cli/src/config/secret.rs
Copilot AI lite review requested due to automatic review settings October 14, 2026 00:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Fix Unicode-aware secret detection to prevent potential credential leakage.

0 open findings

1 resolved since last review

🧠 Review effort: Lite

@fnando
fnando merged commit 94703b2 into main Oct 14, 2026
147 checks passed
@fnando
fnando deleted the conceal-mistyped-secret-in-contract-id branch October 14, 2026 00:48
@github-project-automation github-project-automation Bot moved this from Needs Review to Done in DevX Oct 14, 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.

4 participants