`keys rm --force` Deletes the Global Alias After Resolving the Local One

[fnando]

007: keys rm --force Deletes the Global Alias After Resolving the Local One

Date: 2026-04-17
Severity: Medium
Impact: unintended file write outside expected paths
Subsystem: keys
Final review by: gpt-5.4, high

Summary

stellar keys rm <name> --force can remove the wrong identity file when the same alias exists in both deprecated local config and the global profile. Resolution is local-first, but deletion is hard-wired to the global config directory, so the local alias remains while the different global alias is deleted.

Root Cause

keys rm only enforces the local-storage safety check on the non---force path. Under --force, locator::Args::remove_identity reads the identity through read_identity (which searches local before global) and then calls KeyType::Identity.remove(name, &self.config_dir()?), where config_dir() always resolves to the global or explicitly selected config directory.

Reproduction

This manifests when a user has a deprecated project-local .stellar/identity/alice.toml and a different global identity/alice.toml at the same time. Running stellar keys rm alice --force from the project directory resolves alice from the local config but deletes the global file instead.

Affected Code

• cmd/soroban-cli/src/commands/keys/rm.rs:37-65 — --force skips the local-storage guard and goes straight to remove_identity
• cmd/soroban-cli/src/config/locator.rs:157-166 — local/global lookup order is local first, but config_dir() returns the global config directory
• cmd/soroban-cli/src/config/locator.rs:322-332 — removal reads one identity and deletes by a different directory source
• cmd/soroban-cli/src/config/locator.rs:647-664 — local identities win during read_with_global_with_location
• cmd/soroban-cli/src/config/locator.rs:782-790 — file deletion happens at the path built from the global config directory

PoC

• Target test file: poc/force-remove-local-global-mismatch
• Test name: force-remove-local-global-mismatch
• Test language: bash
• How to run:

1. Run cargo build from the repo root.
2. Copy the script below to poc/force-remove-local-global-mismatch.
3. Run: bash poc/force-remove-local-global-mismatch

Test Body

#!/usr/bin/env bash
set -euo pipefail

STELLAR="$(cd /repo/stellar-cli && pwd)/target/debug/stellar"

WORKDIR=$(mktemp -d)
GLOBALDIR=$(mktemp -d)
trap 'rm -rf "$WORKDIR" "$GLOBALDIR"' EXIT

LOCAL_ID_DIR="$WORKDIR/.stellar/identity"
GLOBAL_ID_DIR="$GLOBALDIR/identity"
mkdir -p "$LOCAL_ID_DIR" "$GLOBAL_ID_DIR"

cat > "$LOCAL_ID_DIR/alice.toml" <<'EOF'
seed_phrase = "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about"
EOF

cat > "$GLOBAL_ID_DIR/alice.toml" <<'EOF'
seed_phrase = "zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo wrong"
EOF

echo "=== Before rm --force ==="
echo "LOCAL  exists: $(test -f "$LOCAL_ID_DIR/alice.toml" && echo YES || echo NO)"
echo "GLOBAL exists: $(test -f "$GLOBAL_ID_DIR/alice.toml" && echo YES || echo NO)"

RESOLVED_PUBKEY=$(cd "$WORKDIR" && STELLAR_CONFIG_HOME="$GLOBALDIR" "$STELLAR" keys public-key alice 2>/dev/null || true)
echo "Resolved public key (should be from LOCAL seed): $RESOLVED_PUBKEY"

cd "$WORKDIR"
STELLAR_CONFIG_HOME="$GLOBALDIR" "$STELLAR" keys rm alice --force 2>/dev/null || true

echo
echo "=== After rm --force ==="
LOCAL_EXISTS=$(test -f "$LOCAL_ID_DIR/alice.toml" && echo YES || echo NO)
GLOBAL_EXISTS=$(test -f "$GLOBAL_ID_DIR/alice.toml" && echo YES || echo NO)
echo "LOCAL  exists: $LOCAL_EXISTS"
echo "GLOBAL exists: $GLOBAL_EXISTS"

PASS=true

if [ "$LOCAL_EXISTS" = "YES" ]; then
    echo "PASS: Local alice.toml was not deleted"
else
    echo "FAIL: Local alice.toml was unexpectedly deleted"
    PASS=false
fi

if [ "$GLOBAL_EXISTS" = "NO" ]; then
    echo "PASS: Global alice.toml was deleted instead"
else
    echo "FAIL: Global alice.toml still exists"
    PASS=false
fi

if [ "$PASS" = true ]; then
    echo "=== PoC CONFIRMED ==="
    exit 0
else
    echo "=== PoC NOT CONFIRMED ==="
    exit 1
fi

Expected vs Actual Behavior

• Expected: keys rm --force should remove the same resolved identity, or reject deprecated local identities even when --force is used.
• Actual: the command resolves the local alias, leaves that local file in place, and deletes the different global alias file.

Adversarial Review

1. Exercises claimed bug: YES — the PoC proves local-first resolution with keys public-key and then shows keys rm --force deletes only the global file.
2. Realistic preconditions: YES — the CLI still supports deprecated local config, duplicate local/global aliases are visible via keys ls --long, and --force is documented only as skipping confirmation.
3. Bug vs by-design: BUG — bypassing the local-storage safeguard changes the deletion target, not just the prompt behavior promised by --force.
4. Final severity: Medium — this is an unintended deletion of a different same-user config file in another config scope.
5. In scope: YES — the behavior is concrete, reproducible, and does not rely on privileged machine access.
6. Test correctness: CORRECT — it checks the real filesystem side effects on both candidate targets and does not rely on tautological assertions.
7. Alternative explanations: NONE
8. Novelty: NOVEL

Suggested Fix

Preserve the resolved location all the way through removal. keys rm should either keep rejecting Location::Local identities even with --force, or pass the resolved location from read_identity_with_location into deletion so the same alias file that was read is the one that gets removed.

[fnando]

Amendment: keys add/generate --overwrite Leaves a Shadowing Local Alias Active

Why This Is Related

This finding shares the same root cause as the original filing: in locator.rs, the read path (read_identity()) searches local_and_global() in local-first order, but subsequent mutation operations use config_dir(), which resolves only to the global config root. In the original issue that mismatch causes the wrong file to be deleted; here the same mismatch causes the wrong scope to be written, leaving the local alias unmodified and still shadowing the newly written global identity. Both deficiencies are remedied by the same fix: carrying the resolved Location through to the mutation call (read_identity_with_location exists but is unused by both code paths).

New Finding

008: keys add/generate --overwrite Leaves a Shadowing Local Alias Active

Date: 2026-04-17
Severity: Medium
Impact: signing with stale or attacker-chosen local alias
Subsystem: keys
Final review by: gpt-5.4, high

Summary

stellar keys generate <name> --overwrite and stellar keys add <name> --overwrite can report a successful overwrite while leaving a deprecated project-local alias active. The existence check resolves identities local-first, but the subsequent write always targets the global config directory, so later address resolution and signing in that directory still use the stale local key.

Root Cause

Both overwrite paths call locator::Args::read_identity() only to decide whether the alias exists. That read path searches local_and_global() in local-first order, while write_identity() and write_key() always write through config_dir(), which resolves only to the global config root. The command therefore "overwrites" a different storage scope than the one it just found.

Reproduction

Create a deprecated .stellar/identity/<alias>.toml in a project directory, point STELLAR_CONFIG_HOME at a separate global profile, and run keys generate --overwrite or keys add --overwrite from the project directory. The CLI prints an overwrite message and creates a new global identity file, but stellar keys address <alias> from the same directory still resolves to the untouched local file.

Affected Code

• cmd/soroban-cli/src/commands/keys/add.rs:60-79 — checks existence with read_identity() and then writes with write_key()
• cmd/soroban-cli/src/commands/keys/generate.rs:70-83 — same overwrite gate followed by write_identity()
• cmd/soroban-cli/src/config/locator.rs:157-198 — local/global lookup order differs from the fixed global write target
• cmd/soroban-cli/src/config/locator.rs:278-287 — read_identity_with_location() exists but is not used by the overwrite commands
• cmd/soroban-cli/src/config/locator.rs:647-664 — identity lookup returns the first hit from local-first search order

PoC

• Target test file: poc/overwrite-shadowed-local-alias
• Test name: overwrite-shadowed-local-alias
• Test language: bash
• How to run:

1. Run cargo build from the repo root.
2. Copy the script below to poc/overwrite-shadowed-local-alias.
3. Run: bash poc/overwrite-shadowed-local-alias

Test Body

#!/usr/bin/env bash
set -euo pipefail

STELLAR="$(cd /repo/stellar-cli && pwd)/target/debug/stellar"

TMPDIR="$(mktemp -d)"
trap 'rm -rf "$TMPDIR"' EXIT

WORKDIR="$TMPDIR/project"
GLOBAL_CONFIG="$TMPDIR/global-config"
SETUP_CONFIG="$TMPDIR/setup-config"

mkdir -p "$WORKDIR/.stellar/identity" "$GLOBAL_CONFIG" "$SETUP_CONFIG"

make_identity() {
    local alias="$1"
    STELLAR_CONFIG_HOME="$SETUP_CONFIG" "$STELLAR" keys generate "$alias" >/dev/null 2>&1
    STELLAR_CONFIG_HOME="$SETUP_CONFIG" "$STELLAR" keys secret "$alias" 2>/dev/null
}

read_pubkey() {
    local alias="$1"
    shift
    "$STELLAR" keys address "$alias" "$@" 2>/dev/null
}

OLD_SECRET="$(make_identity old-local)"
OLD_PUBKEY="$(STELLAR_CONFIG_HOME="$SETUP_CONFIG" read_pubkey old-local)"

NEW_ADD_SECRET="$(make_identity new-add)"
NEW_ADD_PUBKEY="$(STELLAR_CONFIG_HOME="$SETUP_CONFIG" read_pubkey new-add)"

printf 'secret_key = "%s"\n' "$OLD_SECRET" > "$WORKDIR/.stellar/identity/alice.toml"
printf 'secret_key = "%s"\n' "$OLD_SECRET" > "$WORKDIR/.stellar/identity/bob.toml"

cd "$WORKDIR"
export STELLAR_CONFIG_HOME="$GLOBAL_CONFIG"

echo "=== Case 1: keys generate --overwrite ==="
echo "Resolved before overwrite: $(read_pubkey alice)"
GENERATE_OUTPUT="$("$STELLAR" keys generate alice --overwrite 2>&1)"
echo "$GENERATE_OUTPUT"
ALICE_GLOBAL_PUBKEY="$(read_pubkey alice --config-dir "$GLOBAL_CONFIG")"
ALICE_RESOLVED_PUBKEY="$(read_pubkey alice)"
echo "Old local key:          $OLD_PUBKEY"
echo "New generated key:      $ALICE_GLOBAL_PUBKEY"
echo "Resolved after write:   $ALICE_RESOLVED_PUBKEY"
echo

echo "=== Case 2: keys add --overwrite ==="
echo "Resolved before overwrite: $(read_pubkey bob)"
ADD_OUTPUT="$(STELLAR_SECRET_KEY="$NEW_ADD_SECRET" "$STELLAR" keys add bob --overwrite 2>&1)"
echo "$ADD_OUTPUT"
BOB_GLOBAL_PUBKEY="$(read_pubkey bob --config-dir "$GLOBAL_CONFIG")"
BOB_RESOLVED_PUBKEY="$(read_pubkey bob)"
echo "Old local key:          $OLD_PUBKEY"
echo "New added key:          $BOB_GLOBAL_PUBKEY"
echo "Expected add key:       $NEW_ADD_PUBKEY"
echo "Resolved after write:   $BOB_RESOLVED_PUBKEY"
echo

PASS=true

if [[ "$ALICE_RESOLVED_PUBKEY" == "$OLD_PUBKEY" && "$ALICE_RESOLVED_PUBKEY" != "$ALICE_GLOBAL_PUBKEY" ]]; then
    echo "PASS: generate --overwrite leaves the local alias active"
else
    echo "FAIL: generate --overwrite did not demonstrate the mismatch"
    PASS=false
fi

if [[ "$BOB_GLOBAL_PUBKEY" == "$NEW_ADD_PUBKEY" && "$BOB_RESOLVED_PUBKEY" == "$OLD_PUBKEY" && "$BOB_RESOLVED_PUBKEY" != "$BOB_GLOBAL_PUBKEY" ]]; then
    echo "PASS: add --overwrite leaves the local alias active"
else
    echo "FAIL: add --overwrite did not demonstrate the mismatch"
    PASS=false
fi

if [[ "$PASS" == true ]]; then
    echo "=== PoC CONFIRMED ==="
    exit 0
fi

echo "=== PoC NOT CONFIRMED ==="
exit 1

Expected vs Actual Behavior

• Expected: --overwrite should replace the identity that satisfied the existence check, or reject the operation when the only matching alias lives in deprecated local storage.
• Actual: the command reports success, writes a new global identity file, and still resolves the old local alias for later keys address and signing operations in that directory.

Adversarial Review

1. Exercises claimed bug: YES — the PoC independently demonstrates the mismatch for both generate --overwrite and add --overwrite, then proves the alias still resolves to the pre-existing local key instead of the newly written global one.
2. Realistic preconditions: YES — local .stellar directories are still auto-discovered via ancestor search and remain active with only a deprecation warning, so a user can encounter this in an existing project checkout.
3. Bug vs by-design: BUG — the command explicitly reports "Overwriting identity" for the alias it just found, but it mutates a different storage scope and leaves the resolved alias unchanged.
4. Final severity: Medium — the bug can keep a stale or attacker-chosen signing identity bound to a trusted alias after an apparent rotation, but it depends on deprecated local config being present.
5. In scope: YES — this is a concrete, user-triggerable behavior with a direct code path and no privileged access requirement.
6. Test correctness: CORRECT — the PoC checks both the global alias that was newly written and the alias actually resolved from the working directory, so the passing condition cannot be explained by tautological setup.
7. Alternative explanations: NONE
8. Novelty: NOVEL — this is distinct from the confirmed keys rm --force local/global mismatch because it targets overwrite flows and post-rotation alias resolution.

Suggested Fix

Carry the resolved location through the overwrite path. keys add and keys generate should use read_identity_with_location() and either overwrite that exact location or reject Location::Local identities with a migration-focused error instead of silently writing a shadowed global alias.