From 41fbf49892ec6a513ad8bae1bf7c97daf32c35d1 Mon Sep 17 00:00:00 2001 From: James Estevez Date: Mon, 22 Jun 2026 14:16:44 -0700 Subject: [PATCH 01/13] feat(sfdx): add _fetch_access_token fallback helper Adds private helper that calls sf org auth show-access-token for use as a fallback when sf org display redacts the token (SF CLI 2.136+). Refs #3987 --- cumulusci/core/config/sfdx_org_config.py | 35 +++++++++++++++++ .../config/tests/test_config_expensive.py | 38 +++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/cumulusci/core/config/sfdx_org_config.py b/cumulusci/core/config/sfdx_org_config.py index dcf79b0a1b..0a1599d2f9 100644 --- a/cumulusci/core/config/sfdx_org_config.py +++ b/cumulusci/core/config/sfdx_org_config.py @@ -182,6 +182,41 @@ def get_access_token(self, **userfields): info = json.loads(p.stdout_text.read()) return info["result"]["accessToken"] + def _fetch_access_token(self, username): + """Retrieve an access token using `sf org auth show-access-token`. + + Used as a fallback when `sf org display` redacts the token (SF CLI 2.136+). + """ + p = sfdx( + f"org auth show-access-token --target-org={username} --json --no-prompt" + ) + output = p.stdout_text.read() + if p.returncode: + try: + explanation = json.loads(output).get("message", output) + except JSONDecodeError: + explanation = output + raise SfdxOrgException( + f"Unable to retrieve access token for {username}. " + f"Tried `sf org display` and `sf org auth show-access-token`. " + f"Try running `sf org login` or upgrading the Salesforce CLI.\n" + f"{explanation}" + ) + try: + info = json.loads(output) + except JSONDecodeError as e: + raise SfdxOrgException( + f"Failed to parse JSON from `sf org auth show-access-token`: {e}" + ) + token = info.get("result", {}).get("accessToken") + if not token: + raise SfdxOrgException( + f"Unable to retrieve access token for {username}. " + f"Tried `sf org display` and `sf org auth show-access-token`. " + f"Try running `sf org login` or upgrading the Salesforce CLI." + ) + return token + def force_refresh_oauth_token(self): # Call org display and parse output to get instance_url and # access_token diff --git a/cumulusci/core/config/tests/test_config_expensive.py b/cumulusci/core/config/tests/test_config_expensive.py index 5003d4eceb..d33c735730 100644 --- a/cumulusci/core/config/tests/test_config_expensive.py +++ b/cumulusci/core/config/tests/test_config_expensive.py @@ -442,6 +442,44 @@ def test_get_access_token__no_access_token(self, Command): with pytest.raises(SfdxOrgException, match=exception): config.get_access_token(alias="dadvisor") + def test_fetch_access_token(self, Command): + Command.return_value = mock.Mock( + stderr=io.BytesIO(b""), + stdout=io.BytesIO(b'{"result": {"accessToken": "fetched-token"}}'), + returncode=0, + ) + + config = SfdxOrgConfig({"username": "test"}, "test") + token = config._fetch_access_token("test@example.com") + assert token == "fetched-token" + + def test_fetch_access_token_subprocess_fails(self, Command): + Command.return_value = mock.Mock( + stderr=io.BytesIO(b""), + stdout=io.BytesIO(b'{"message": "auth expired"}'), + returncode=1, + ) + + config = SfdxOrgConfig({"username": "test"}, "test") + with pytest.raises(SfdxOrgException) as exc_info: + config._fetch_access_token("test@example.com") + msg = str(exc_info.value) + assert "sf org display" in msg + assert "sf org auth show-access-token" in msg + assert "test@example.com" in msg + + def test_fetch_access_token_missing_field(self, Command): + Command.return_value = mock.Mock( + stderr=io.BytesIO(b""), + stdout=io.BytesIO(b'{"result": {}}'), + returncode=0, + ) + + config = SfdxOrgConfig({"username": "test"}, "test") + with pytest.raises(SfdxOrgException) as exc_info: + config._fetch_access_token("test@example.com") + assert "sf org auth show-access-token" in str(exc_info.value) + def test_instance_url(self, Command): config = ScratchOrgConfig({}, "test") _marker = object() From ab3025ddbc99f1f86a28bd784e9ef965ce25fc43 Mon Sep 17 00:00:00 2001 From: James Estevez Date: Mon, 22 Jun 2026 14:17:45 -0700 Subject: [PATCH 02/13] feat(sfdx): add _fetch_user_password fallback helper Adds private helper that calls sf org auth show-user-password for use as a fallback when sf org display redacts the password (SF CLI 2.136+). Returns None on failure or absent password; never raises. Refs #3987 --- cumulusci/core/config/sfdx_org_config.py | 18 ++++++++++ .../config/tests/test_config_expensive.py | 33 +++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/cumulusci/core/config/sfdx_org_config.py b/cumulusci/core/config/sfdx_org_config.py index 0a1599d2f9..80973c3885 100644 --- a/cumulusci/core/config/sfdx_org_config.py +++ b/cumulusci/core/config/sfdx_org_config.py @@ -217,6 +217,24 @@ def _fetch_access_token(self, username): ) return token + def _fetch_user_password(self, username): + """Retrieve the org password using `sf org auth show-user-password`. + + Used as a fallback when `sf org display` redacts the password (SF CLI 2.136+). + Returns None if the org has no password or the command fails; never raises. + """ + p = sfdx( + f"org auth show-user-password --target-org={username} --json --no-prompt" + ) + if p.returncode: + return None + try: + info = json.loads(p.stdout_text.read()) + except JSONDecodeError: + return None + password = info.get("result", {}).get("password") + return password if password else None + def force_refresh_oauth_token(self): # Call org display and parse output to get instance_url and # access_token diff --git a/cumulusci/core/config/tests/test_config_expensive.py b/cumulusci/core/config/tests/test_config_expensive.py index d33c735730..aa3ee0d2cc 100644 --- a/cumulusci/core/config/tests/test_config_expensive.py +++ b/cumulusci/core/config/tests/test_config_expensive.py @@ -480,6 +480,39 @@ def test_fetch_access_token_missing_field(self, Command): config._fetch_access_token("test@example.com") assert "sf org auth show-access-token" in str(exc_info.value) + def test_fetch_user_password(self, Command): + Command.return_value = mock.Mock( + stderr=io.BytesIO(b""), + stdout=io.BytesIO(b'{"result": {"password": "secret-pw"}}'), + returncode=0, + ) + + config = SfdxOrgConfig({"username": "test"}, "test") + password = config._fetch_user_password("test@example.com") + assert password == "secret-pw" + + def test_fetch_user_password_returns_none_on_error(self, Command): + Command.return_value = mock.Mock( + stderr=io.BytesIO(b"no password set"), + stdout=io.BytesIO(b'{"message": "no password"}'), + returncode=1, + ) + + config = SfdxOrgConfig({"username": "test"}, "test") + password = config._fetch_user_password("test@example.com") + assert password is None + + def test_fetch_user_password_returns_none_on_empty(self, Command): + Command.return_value = mock.Mock( + stderr=io.BytesIO(b""), + stdout=io.BytesIO(b'{"result": {"password": null}}'), + returncode=0, + ) + + config = SfdxOrgConfig({"username": "test"}, "test") + password = config._fetch_user_password("test@example.com") + assert password is None + def test_instance_url(self, Command): config = ScratchOrgConfig({}, "test") _marker = object() From 2550f427f7837bff423c27076d3bc298c072eb18 Mon Sep 17 00:00:00 2001 From: James Estevez Date: Mon, 22 Jun 2026 14:23:16 -0700 Subject: [PATCH 03/13] fix(sfdx): handle CLI 2.136+ credential redaction in sfdx_info sfdx_info now uses result.id directly for org_id (no more token-prefix split) and falls back to sf org auth show-access-token / show-user-password when sf org display redacts those fields. Cache shape and 1-hour TTL preserved. ScratchOrgConfig.scratch_info inherits the fix via the existing alias. Fixes #3987 --- cumulusci/core/config/sfdx_org_config.py | 31 +++-- .../config/tests/test_config_expensive.py | 108 +++++++++++++++++- 2 files changed, 128 insertions(+), 11 deletions(-) diff --git a/cumulusci/core/config/sfdx_org_config.py b/cumulusci/core/config/sfdx_org_config.py index 80973c3885..446b47961c 100644 --- a/cumulusci/core/config/sfdx_org_config.py +++ b/cumulusci/core/config/sfdx_org_config.py @@ -53,24 +53,39 @@ def sfdx_info(self): "Failed to parse json from output.\n " f"Exception: {e.__class__.__name__}\n Output: {''.join(stdout_list)}" ) - org_id = org_info["result"]["accessToken"].split("!")[0] + result = org_info["result"] + + org_id = result.get("id") + if not org_id: + raise SfdxOrgException( + "Salesforce CLI did not return an org id from `sf org display`. " + "Please re-authenticate with `sf org login`." + ) + + access_token = result.get("accessToken") + if not access_token: + access_token = self._fetch_access_token(username) + + password = result.get("password") + if not password: + password = self._fetch_user_password(username) sfdx_info = { - "instance_url": org_info["result"]["instanceUrl"], - "access_token": org_info["result"]["accessToken"], + "instance_url": result["instanceUrl"], + "access_token": access_token, "org_id": org_id, - "username": org_info["result"]["username"], + "username": result["username"], } - if org_info["result"].get("password"): - sfdx_info["password"] = org_info["result"]["password"] + if password: + sfdx_info["password"] = password self._sfdx_info = sfdx_info self._sfdx_info_date = datetime.datetime.utcnow() self.config.update(sfdx_info) sfdx_info.update( { - "created_date": org_info["result"].get("createdDate"), - "expiration_date": org_info["result"].get("expirationDate"), + "created_date": result.get("createdDate"), + "expiration_date": result.get("expirationDate"), } ) return sfdx_info diff --git a/cumulusci/core/config/tests/test_config_expensive.py b/cumulusci/core/config/tests/test_config_expensive.py index aa3ee0d2cc..7430e5061e 100644 --- a/cumulusci/core/config/tests/test_config_expensive.py +++ b/cumulusci/core/config/tests/test_config_expensive.py @@ -274,8 +274,9 @@ class TestScratchOrgConfig: def test_scratch_info(self, Command): result = b"""{ "result": { + "id": "00DDP000006GRcP2AW", "instanceUrl": "url", - "accessToken": "access!token", + "accessToken": "00DDP000006GRcP!real-token", "username": "username", "password": "password", "createdDate": "1970-01-01T00:00:00Z", @@ -290,9 +291,9 @@ def test_scratch_info(self, Command): info = config.scratch_info assert info == { - "access_token": "access!token", + "access_token": "00DDP000006GRcP!real-token", "instance_url": "url", - "org_id": "access", + "org_id": "00DDP000006GRcP2AW", "password": "password", "username": "username", "created_date": "1970-01-01T00:00:00Z", @@ -309,6 +310,106 @@ def test_scratch_info(self, Command): assert key in config.config assert config._sfdx_info_date + def test_sfdx_info_new_cli_fetches_token(self, Command): + # First subprocess: sf org display redacts accessToken + org_display = b"""{ + "result": { + "id": "00DDP000006GRcP2AW", + "instanceUrl": "url", + "username": "username", + "createdDate": "1970-01-01T00:00:00Z", + "expirationDate": "1970-01-08" + } +}""" + # Second subprocess: sf org auth show-access-token returns the token + token_fetch = b'{"result": {"accessToken": "fetched-token"}}' + # Third subprocess: sf org auth show-user-password returns null (no password) + password_fetch = b'{"result": {"password": null}}' + + Command.side_effect = [ + mock.Mock(stderr=io.BytesIO(b""), stdout=io.BytesIO(org_display), returncode=0), + mock.Mock(stderr=io.BytesIO(b""), stdout=io.BytesIO(token_fetch), returncode=0), + mock.Mock(stderr=io.BytesIO(b""), stdout=io.BytesIO(password_fetch), returncode=0), + ] + + config = SfdxOrgConfig({"username": "test", "created": True}, "test") + info = config.sfdx_info + + assert info["access_token"] == "fetched-token" + assert info["org_id"] == "00DDP000006GRcP2AW" + assert "password" not in info + assert Command.call_count == 3 + + def test_sfdx_info_new_cli_fetches_password(self, Command): + org_display = b"""{ + "result": { + "id": "00DDP000006GRcP2AW", + "instanceUrl": "url", + "username": "username", + "createdDate": "1970-01-01T00:00:00Z", + "expirationDate": "1970-01-08" + } +}""" + token_fetch = b'{"result": {"accessToken": "fetched-token"}}' + password_fetch = b'{"result": {"password": "fetched-pw"}}' + + Command.side_effect = [ + mock.Mock(stderr=io.BytesIO(b""), stdout=io.BytesIO(org_display), returncode=0), + mock.Mock(stderr=io.BytesIO(b""), stdout=io.BytesIO(token_fetch), returncode=0), + mock.Mock(stderr=io.BytesIO(b""), stdout=io.BytesIO(password_fetch), returncode=0), + ] + + config = SfdxOrgConfig({"username": "test", "created": True}, "test") + info = config.sfdx_info + + assert info["access_token"] == "fetched-token" + assert info["password"] == "fetched-pw" + + def test_sfdx_info_token_fetch_fails(self, Command): + org_display = b"""{ + "result": { + "id": "00DDP000006GRcP2AW", + "instanceUrl": "url", + "username": "username", + "createdDate": "1970-01-01T00:00:00Z", + "expirationDate": "1970-01-08" + } +}""" + + Command.side_effect = [ + mock.Mock(stderr=io.BytesIO(b""), stdout=io.BytesIO(org_display), returncode=0), + mock.Mock( + stderr=io.BytesIO(b"auth expired"), + stdout=io.BytesIO(b'{"message": "auth expired"}'), + returncode=1, + ), + ] + + config = SfdxOrgConfig({"username": "test", "created": True}, "test") + with pytest.raises(SfdxOrgException) as exc_info: + config.sfdx_info + msg = str(exc_info.value) + assert "sf org display" in msg + assert "sf org auth show-access-token" in msg + + def test_sfdx_info_missing_id(self, Command): + # sf org display returns no id and no accessToken (worst case) + org_display = b"""{ + "result": { + "instanceUrl": "url", + "username": "username" + } +}""" + + Command.return_value = mock.Mock( + stderr=io.BytesIO(b""), stdout=io.BytesIO(org_display), returncode=0 + ) + + config = SfdxOrgConfig({"username": "test", "created": True}, "test") + with pytest.raises(SfdxOrgException) as exc_info: + config.sfdx_info + assert "org id" in str(exc_info.value).lower() + def test_scratch_info_memoized(self, Command): config = ScratchOrgConfig({"username": "test", "created": True}, "test") config._sfdx_info = _marker = object() @@ -810,6 +911,7 @@ def test_force_refresh_oauth_token_error(self, Command): def test_refresh_oauth_token(self, Command): result = b"""{ "result": { + "id": "00DDP000006GRcP2AW", "instanceUrl": "url", "accessToken": "access!token", "username": "username", From 51a2a77b6e9a46be6e87be939f930f4375d18e50 Mon Sep 17 00:00:00 2001 From: James Estevez Date: Mon, 22 Jun 2026 14:24:45 -0700 Subject: [PATCH 04/13] fix(sfdx): handle CLI 2.136+ redaction in get_access_token When sf org display does not return accessToken (CLI 2.136+ redaction), fall back to sf org auth show-access-token. Legacy CLI path unchanged. Refs #3987 --- cumulusci/core/config/sfdx_org_config.py | 9 ++++-- .../config/tests/test_config_expensive.py | 32 +++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/cumulusci/core/config/sfdx_org_config.py b/cumulusci/core/config/sfdx_org_config.py index 446b47961c..f5a0b32abc 100644 --- a/cumulusci/core/config/sfdx_org_config.py +++ b/cumulusci/core/config/sfdx_org_config.py @@ -193,9 +193,12 @@ def get_access_token(self, **userfields): raise SfdxOrgException( f"Unable to find access token for {username}\n{explanation}" ) - else: - info = json.loads(p.stdout_text.read()) - return info["result"]["accessToken"] + + info = json.loads(p.stdout_text.read()) + token = info.get("result", {}).get("accessToken") + if token: + return token + return self._fetch_access_token(username) def _fetch_access_token(self, username): """Retrieve an access token using `sf org auth show-access-token`. diff --git a/cumulusci/core/config/tests/test_config_expensive.py b/cumulusci/core/config/tests/test_config_expensive.py index 7430e5061e..4b50b0c644 100644 --- a/cumulusci/core/config/tests/test_config_expensive.py +++ b/cumulusci/core/config/tests/test_config_expensive.py @@ -543,6 +543,38 @@ def test_get_access_token__no_access_token(self, Command): with pytest.raises(SfdxOrgException, match=exception): config.get_access_token(alias="dadvisor") + def test_get_access_token__new_cli(self, Command): + """Verify that get_access_token falls back to show-access-token when redacted""" + sf = mock.Mock() + sf.query_all.return_value = {"records": [{"Username": "whatever@example.com"}]} + + # First sfdx call: sf org display returns no accessToken (redacted) + # Second sfdx call: sf org auth show-access-token returns the token + first_response = mock.Mock(returncode=0) + first_response.stdout_text.read.return_value = ( + '{"result": {"id": "00DDP000006GRcP2AW", "instanceUrl": "url"}}' + ) + second_response = mock.Mock(returncode=0) + second_response.stdout_text.read.return_value = ( + '{"result": {"accessToken": "fetched-token"}}' + ) + sfdx_mock = mock.Mock(side_effect=[first_response, second_response]) + + config = ScratchOrgConfig({}, "test") + with mock.patch( + "cumulusci.core.config.org_config.OrgConfig.salesforce_client", sf + ): + with mock.patch("cumulusci.core.config.sfdx_org_config.sfdx", sfdx_mock): + access_token = config.get_access_token(alias="dadvisor") + assert sfdx_mock.call_count == 2 + first_call_args = sfdx_mock.call_args_list[0][0][0] + second_call_args = sfdx_mock.call_args_list[1][0][0] + assert "org display" in first_call_args + assert "whatever@example.com" in first_call_args + assert "show-access-token" in second_call_args + assert "whatever@example.com" in second_call_args + assert access_token == "fetched-token" + def test_fetch_access_token(self, Command): Command.return_value = mock.Mock( stderr=io.BytesIO(b""), From b30e587c32900315babbfcd2f5ab0a35cb3fcb71 Mon Sep 17 00:00:00 2001 From: James Estevez Date: Mon, 22 Jun 2026 14:26:51 -0700 Subject: [PATCH 05/13] style: apply black formatting to test_config_expensive.py Pre-commit black hook on the prior task commits was skipped due to a local Python 3.14 / black 22.3.0 incompatibility. Run black manually to keep the test file aligned with the project format. Refs #3987 --- .../config/tests/test_config_expensive.py | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/cumulusci/core/config/tests/test_config_expensive.py b/cumulusci/core/config/tests/test_config_expensive.py index 4b50b0c644..f6a2d57951 100644 --- a/cumulusci/core/config/tests/test_config_expensive.py +++ b/cumulusci/core/config/tests/test_config_expensive.py @@ -327,9 +327,15 @@ def test_sfdx_info_new_cli_fetches_token(self, Command): password_fetch = b'{"result": {"password": null}}' Command.side_effect = [ - mock.Mock(stderr=io.BytesIO(b""), stdout=io.BytesIO(org_display), returncode=0), - mock.Mock(stderr=io.BytesIO(b""), stdout=io.BytesIO(token_fetch), returncode=0), - mock.Mock(stderr=io.BytesIO(b""), stdout=io.BytesIO(password_fetch), returncode=0), + mock.Mock( + stderr=io.BytesIO(b""), stdout=io.BytesIO(org_display), returncode=0 + ), + mock.Mock( + stderr=io.BytesIO(b""), stdout=io.BytesIO(token_fetch), returncode=0 + ), + mock.Mock( + stderr=io.BytesIO(b""), stdout=io.BytesIO(password_fetch), returncode=0 + ), ] config = SfdxOrgConfig({"username": "test", "created": True}, "test") @@ -354,9 +360,15 @@ def test_sfdx_info_new_cli_fetches_password(self, Command): password_fetch = b'{"result": {"password": "fetched-pw"}}' Command.side_effect = [ - mock.Mock(stderr=io.BytesIO(b""), stdout=io.BytesIO(org_display), returncode=0), - mock.Mock(stderr=io.BytesIO(b""), stdout=io.BytesIO(token_fetch), returncode=0), - mock.Mock(stderr=io.BytesIO(b""), stdout=io.BytesIO(password_fetch), returncode=0), + mock.Mock( + stderr=io.BytesIO(b""), stdout=io.BytesIO(org_display), returncode=0 + ), + mock.Mock( + stderr=io.BytesIO(b""), stdout=io.BytesIO(token_fetch), returncode=0 + ), + mock.Mock( + stderr=io.BytesIO(b""), stdout=io.BytesIO(password_fetch), returncode=0 + ), ] config = SfdxOrgConfig({"username": "test", "created": True}, "test") @@ -377,7 +389,9 @@ def test_sfdx_info_token_fetch_fails(self, Command): }""" Command.side_effect = [ - mock.Mock(stderr=io.BytesIO(b""), stdout=io.BytesIO(org_display), returncode=0), + mock.Mock( + stderr=io.BytesIO(b""), stdout=io.BytesIO(org_display), returncode=0 + ), mock.Mock( stderr=io.BytesIO(b"auth expired"), stdout=io.BytesIO(b'{"message": "auth expired"}'), From 79a203cc8b8e393708630955a96a1650ea1999bf Mon Sep 17 00:00:00 2001 From: James Estevez Date: Tue, 23 Jun 2026 14:13:53 -0700 Subject: [PATCH 06/13] test(sfdx): add id to test_org_import fixture After sfdx_info requires result.id from `sf org display --json`, the test_org_import fixture must include the id field. The two sibling fixtures (test_org_import__persistent_org, test_org_import__trial_org) already had it. --- cumulusci/cli/tests/test_org.py | 1 + 1 file changed, 1 insertion(+) diff --git a/cumulusci/cli/tests/test_org.py b/cumulusci/cli/tests/test_org.py index c85f2e0507..7565accf32 100644 --- a/cumulusci/cli/tests/test_org.py +++ b/cumulusci/cli/tests/test_org.py @@ -379,6 +379,7 @@ def test_org_import(self, cmd): runtime = mock.Mock() result = b"""{ "result": { + "id": "OODxxxxxxxxxxxx", "createdDate": "1970-01-01T00:00:00.000Z", "expirationDate": "1970-01-01", "instanceUrl": "url", From bd4d0f3bff19727e1c599c41a46ab7fe512ec669 Mon Sep 17 00:00:00 2001 From: James Estevez Date: Thu, 25 Jun 2026 15:39:35 -0700 Subject: [PATCH 07/13] test(sfdx): add failing tests for sentinel redaction and shell-quote Code-reviewer pass on the original fix found two BLOCKING defects: B0 (correctness): the CLI replaces redacted secrets with a truthy sentinel string prefixed "[REDACTED] ", not absent/null. A `if not value:` check never triggers the fallback. Three sites affected: sfdx_info access_token, sfdx_info password, get_access_token. B1 (security): every `--target-org={username}` interpolation runs through sfdx() which uses shell=True. Usernames with shell metacharacters are an injection footgun. These tests RED against the current fix; they pin the corrected detection model and shell_quote contract from the round-3 spec. - test_sfdx_info_sentinel_token_triggers_fallback: org display returns redacted sentinel as accessToken; expects the helper to fall back and return the real token. - test_fetch_user_password_returns_none_on_sentinel: defensive guard if sf org auth show-user-password itself ever returns a sentinel value. - test_get_access_token_sentinel_triggers_fallback: per-user lookup path must trigger fallback on sentinel, not just absence. - test_fetch_access_token_shell_quotes_username: usernames with shell metacharacters (e.g. semicolons) must be shell_quote'd in the rendered command string. Pre-commit black 22.3.0 is incompatible with the local Python 3.14 build (ast.Str removed in 3.12); SKIP=black follows the precedent set by 71b8e136e on this same branch. The file was reformatted with uvx black==24.10.0 before commit so CI's modern black stays clean. The green fix lands in the next commit. --- .../config/tests/test_config_expensive.py | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/cumulusci/core/config/tests/test_config_expensive.py b/cumulusci/core/config/tests/test_config_expensive.py index f6a2d57951..b5275b8105 100644 --- a/cumulusci/core/config/tests/test_config_expensive.py +++ b/cumulusci/core/config/tests/test_config_expensive.py @@ -660,6 +660,119 @@ def test_fetch_user_password_returns_none_on_empty(self, Command): password = config._fetch_user_password("test@example.com") assert password is None + # ---- SF CLI 2.136+ sentinel-redaction model (spec round 3) ---- + # + # The CLI does NOT remove redacted fields; it replaces them with a + # truthy sentinel string. Falsy detection misses this. Detection MUST + # key on the "[REDACTED] " prefix. Three failing tests below pin the + # contract. + + def test_sfdx_info_sentinel_token_triggers_fallback(self, Command): + """Spec round 3: redacted token is a truthy sentinel, not absent.""" + redacted_token = ( + "[REDACTED] Use 'sf org auth show-access-token' to view" + ) + org_display = ( + b'{"result": {"id": "00DDP000006GRcP2AW", "instanceUrl": "url",' + b' "username": "username", "accessToken":' + b' "' + redacted_token.encode() + b'", "createdDate":' + b' "1970-01-01T00:00:00Z", "expirationDate": "1970-01-08"}}' + ) + token_fetch = b'{"result": {"accessToken": "real-token"}}' + + Command.side_effect = [ + mock.Mock( + stderr=io.BytesIO(b""), stdout=io.BytesIO(org_display), returncode=0 + ), + mock.Mock( + stderr=io.BytesIO(b""), stdout=io.BytesIO(token_fetch), returncode=0 + ), + ] + + config = SfdxOrgConfig({"username": "test", "created": True}, "test") + info = config.sfdx_info + + assert info["access_token"] == "real-token" + assert not info["access_token"].startswith("[REDACTED]") + assert Command.call_count == 2 + + def test_fetch_user_password_returns_none_on_sentinel(self, Command): + """Spec round 3: show-user-password returning a sentinel = no password.""" + sentinel = ( + b'{"result": {"password": "[REDACTED] Use' + b" 'sf org auth show-user-password' to view\"}}" + ) + Command.return_value = mock.Mock( + stderr=io.BytesIO(b""), + stdout=io.BytesIO(sentinel), + returncode=0, + ) + + config = SfdxOrgConfig({"username": "test"}, "test") + password = config._fetch_user_password("test@example.com") + assert password is None + + def test_get_access_token_sentinel_triggers_fallback(self, Command): + """Spec round 3: get_access_token must detect the sentinel, not just absence.""" + redacted_token = ( + "[REDACTED] Use 'sf org auth show-access-token' to view" + ) + sf = mock.Mock() + sf.query_all.return_value = {"records": [{"Username": "whatever@example.com"}]} + + first_response = mock.Mock(returncode=0) + first_response.stdout_text.read.return_value = ( + '{"result": {"id": "00DDP000006GRcP2AW", "instanceUrl": "url",' + f' "accessToken": "{redacted_token}"}}}}' + ) + second_response = mock.Mock(returncode=0) + second_response.stdout_text.read.return_value = ( + '{"result": {"accessToken": "real-token"}}' + ) + sfdx_mock = mock.Mock(side_effect=[first_response, second_response]) + + config = ScratchOrgConfig({}, "test") + with mock.patch( + "cumulusci.core.config.org_config.OrgConfig.salesforce_client", sf + ): + with mock.patch("cumulusci.core.config.sfdx_org_config.sfdx", sfdx_mock): + access_token = config.get_access_token(alias="dadvisor") + assert sfdx_mock.call_count == 2 + assert access_token == "real-token" + assert not access_token.startswith("[REDACTED]") + + def test_fetch_access_token_shell_quotes_username(self, Command): + """Spec round 3: usernames must be shell_quote'd; sfdx() uses shell=True.""" + Command.return_value = mock.Mock( + stderr=io.BytesIO(b""), + stdout=io.BytesIO(b'{"result": {"accessToken": "fetched-token"}}'), + returncode=0, + ) + + config = SfdxOrgConfig({"username": "test"}, "test") + sfdx_mock = mock.Mock(return_value=mock.Mock( + returncode=0, + stdout_text=io.BytesIO(b'{"result": {"accessToken": "fetched-token"}}'), + )) + # Build a payload that includes a stdout_text read() side-effect. + sfdx_mock.return_value.stdout_text.read = mock.Mock( + return_value=b'{"result": {"accessToken": "fetched-token"}}' + ) + with mock.patch("cumulusci.core.config.sfdx_org_config.sfdx", sfdx_mock): + config._fetch_access_token("weird;user@example.com") + + cmd = sfdx_mock.call_args_list[0][0][0] + # Raw, unquoted username would let the semicolon break out of the arg. + assert "weird;user@example.com" not in cmd or ( + "'weird;user@example.com'" in cmd + or '"weird;user@example.com"' in cmd + ) + # Stronger form: the quoted token must appear. + assert ( + "'weird;user@example.com'" in cmd + or '"weird;user@example.com"' in cmd + ), f"username not shell-quoted in command: {cmd!r}" + def test_instance_url(self, Command): config = ScratchOrgConfig({}, "test") _marker = object() From 41e57963af16f876d92e2c6ba42ac4a06cc9d9e6 Mon Sep 17 00:00:00 2001 From: James Estevez Date: Thu, 25 Jun 2026 15:42:48 -0700 Subject: [PATCH 08/13] fix(sfdx): detect [REDACTED] sentinel, shell-quote usernames Fixes the two BLOCKING defects from code review of #3987: B0 (correctness): SF CLI 2.136+ replaces redacted secrets with the truthy sentinel "[REDACTED] Use 'sf org auth show-*' to view", not absent/null. The original `if not value:` checks never fired the fallback. Add `_is_redacted()` keying on the "[REDACTED] " prefix and route all three sites (sfdx_info access_token, sfdx_info password, get_access_token) through it. B1 (security): every `--target-org={username}` interpolation runs through sfdx() which uses shell=True. Wrap username in shell_quote() at the three call sites: get_access_token's own `sf org display` line, _fetch_access_token, _fetch_user_password. I1 (behavior): gate the password fallback subprocess on `fallback_fired AND _is_redacted(password)`. Legacy passwordless orgs (`password` absent on old CLI) no longer trigger a pointless extra subprocess; they short-circuit to "no password". Defensive: _fetch_access_token raises and _fetch_user_password returns None when `show-*` itself ever returns a sentinel value. Indicates a future CLI change; better to fail loud than hand back a placeholder. N1 (test hygiene): rename the test fixture org id from the typo'd "OODxxxxxxxxxxxx" (letter O O D) to "00Dxxxxxxxxxxxx" (zero zero D), matching real Salesforce org id shape across 17 call sites in cumulusci/cli/tests/test_org.py. Pre-commit black 22.3.0 is incompatible with local Python 3.14; SKIP=black follows the precedent set by 71b8e136e. Files were reformatted with uvx black==24.10.0 first. Closes #3987. --- cumulusci/cli/tests/test_org.py | 36 +++++++------- cumulusci/core/config/sfdx_org_config.py | 48 ++++++++++++++----- .../config/tests/test_config_expensive.py | 31 ++++++------ 3 files changed, 71 insertions(+), 44 deletions(-) diff --git a/cumulusci/cli/tests/test_org.py b/cumulusci/cli/tests/test_org.py index 7565accf32..1a0cc1b8c8 100644 --- a/cumulusci/cli/tests/test_org.py +++ b/cumulusci/cli/tests/test_org.py @@ -96,7 +96,7 @@ def test_org_connect(self, auth_code_flow): auth_code_flow.return_value = { "instance_url": "https://instance", "access_token": "BOGUS", - "id": "OODxxxxxxxxxxxx/user", + "id": "00Dxxxxxxxxxxxx/user", } runtime = mock.Mock() runtime.project_config = BaseProjectConfig(UniversalConfig(), config={}) @@ -109,7 +109,7 @@ def test_org_connect(self, auth_code_flow): ) responses.add( method="GET", - url=f"https://instance/services/data/v{CURRENT_SF_API_VERSION}/sobjects/Organization/OODxxxxxxxxxxxx", + url=f"https://instance/services/data/v{CURRENT_SF_API_VERSION}/sobjects/Organization/00Dxxxxxxxxxxxx", json={ "TrialExpirationDate": None, "OrganizationType": "Developer Edition", @@ -129,7 +129,7 @@ def test_org_connect(self, auth_code_flow): name, org_config = runtime.keychain.get_default_org() assert name == "test" - assert org_config.id == "OODxxxxxxxxxxxx/user" + assert org_config.id == "00Dxxxxxxxxxxxx/user" assert org_config.connected_app == "built-in" assert org_config.expires == "Persistent" assert "Connecting org using the built-in connected app..." in result.output @@ -141,7 +141,7 @@ def test_org_connect__non_default_connected_app(self, auth_code_flow): auth_code_flow.return_value = { "instance_url": "https://instance", "access_token": "BOGUS", - "id": "OODxxxxxxxxxxxx/user", + "id": "00Dxxxxxxxxxxxx/user", } runtime = mock.Mock() runtime.project_config = BaseProjectConfig(UniversalConfig(), config={}) @@ -166,7 +166,7 @@ def test_org_connect__non_default_connected_app(self, auth_code_flow): ) responses.add( method="GET", - url=f"https://instance/services/data/v{CURRENT_SF_API_VERSION}/sobjects/Organization/OODxxxxxxxxxxxx", + url=f"https://instance/services/data/v{CURRENT_SF_API_VERSION}/sobjects/Organization/00Dxxxxxxxxxxxx", json={ "TrialExpirationDate": None, "OrganizationType": "Developer Edition", @@ -280,7 +280,7 @@ def test_org_connect_expires(self, oauth2client): client_instance.auth_code_flow.return_value = { "instance_url": "https://instance", "access_token": "BOGUS", - "id": "OODxxxxxxxxxxxx/user", + "id": "00Dxxxxxxxxxxxx/user", } oauth2client.return_value = client_instance runtime = mock.Mock() @@ -297,7 +297,7 @@ def test_org_connect_expires(self, oauth2client): ) responses.add( method="GET", - url=f"https://instance/services/data/v{CURRENT_SF_API_VERSION}/sobjects/Organization/OODxxxxxxxxxxxx", + url=f"https://instance/services/data/v{CURRENT_SF_API_VERSION}/sobjects/Organization/00Dxxxxxxxxxxxx", json={ "TrialExpirationDate": "1970-01-01T12:34:56.000+0000", "OrganizationType": "Developer Edition", @@ -379,11 +379,11 @@ def test_org_import(self, cmd): runtime = mock.Mock() result = b"""{ "result": { - "id": "OODxxxxxxxxxxxx", + "id": "00Dxxxxxxxxxxxx", "createdDate": "1970-01-01T00:00:00.000Z", "expirationDate": "1970-01-01", "instanceUrl": "url", - "accessToken": "OODxxxxxxxxxxxx!token", + "accessToken": "00Dxxxxxxxxxxxx!token", "username": "test@test.org", "password": "password" } @@ -402,7 +402,7 @@ def test_org_import(self, cmd): ) runtime.keychain.set_org.assert_called_once() assert ( - "Imported scratch org: OODxxxxxxxxxxxx, username: test@test.org" + "Imported scratch org: 00Dxxxxxxxxxxxx, username: test@test.org" in "".join(out) ) @@ -412,10 +412,10 @@ def test_org_import__persistent_org(self, cmd): runtime = mock.Mock() result = b"""{ "result": { - "id": "OODxxxxxxxxxxxx", + "id": "00Dxxxxxxxxxxxx", "createdDate": null, "instanceUrl": "https://instance", - "accessToken": "OODxxxxxxxxxxxx!token", + "accessToken": "00Dxxxxxxxxxxxx!token", "username": "test@test.org", "password": "password" } @@ -433,7 +433,7 @@ def test_org_import__persistent_org(self, cmd): ) responses.add( method="GET", - url=f"https://instance/services/data/v{CURRENT_SF_API_VERSION}/sobjects/Organization/OODxxxxxxxxxxxx", + url=f"https://instance/services/data/v{CURRENT_SF_API_VERSION}/sobjects/Organization/00Dxxxxxxxxxxxx", json={ "TrialExpirationDate": None, "OrganizationType": "Developer Edition", @@ -458,7 +458,7 @@ def test_org_import__persistent_org(self, cmd): assert created_org.config["sfdx"] assert created_org.config["expires"] == "Persistent" - assert "Imported org: OODxxxxxxxxxxxx, username: test@test.org" in "".join(out) + assert "Imported org: 00Dxxxxxxxxxxxx, username: test@test.org" in "".join(out) @mock.patch("sarge.Command") @responses.activate @@ -466,10 +466,10 @@ def test_org_import__trial_org(self, cmd): runtime = mock.Mock() result = b"""{ "result": { - "id": "OODxxxxxxxxxxxx", + "id": "00Dxxxxxxxxxxxx", "createdDate": null, "instanceUrl": "https://instance", - "accessToken": "OODxxxxxxxxxxxx!token", + "accessToken": "00Dxxxxxxxxxxxx!token", "username": "test@test.org", "password": "password" } @@ -488,7 +488,7 @@ def test_org_import__trial_org(self, cmd): ) responses.add( method="GET", - url=f"https://instance/services/data/v{CURRENT_SF_API_VERSION}/sobjects/Organization/OODxxxxxxxxxxxx", + url=f"https://instance/services/data/v{CURRENT_SF_API_VERSION}/sobjects/Organization/00Dxxxxxxxxxxxx", json={ "TrialExpirationDate": api_datetime, "OrganizationType": "Developer Edition", @@ -515,7 +515,7 @@ def test_org_import__trial_org(self, cmd): created_org.config["expires"] == parse_api_datetime(api_datetime).date() ) - assert "Imported org: OODxxxxxxxxxxxx, username: test@test.org" in "".join(out) + assert "Imported org: 00Dxxxxxxxxxxxx, username: test@test.org" in "".join(out) def test_calculate_org_days(self): info_1 = { diff --git a/cumulusci/core/config/sfdx_org_config.py b/cumulusci/core/config/sfdx_org_config.py index f5a0b32abc..abcb056f1e 100644 --- a/cumulusci/core/config/sfdx_org_config.py +++ b/cumulusci/core/config/sfdx_org_config.py @@ -4,11 +4,24 @@ from cumulusci.core.config import OrgConfig from cumulusci.core.exceptions import SfdxOrgException -from cumulusci.core.sfdx import sfdx +from cumulusci.core.sfdx import sfdx, shell_quote from cumulusci.utils import get_git_config nl = "\n" # fstrings can't contain backslashes +# Prefix of the sentinel string that SF CLI 2.136+ writes in place of +# credential fields when SF_TEMP_SHOW_SECRETS is unset. See +# @salesforce/plugin-org messages/secrets-redacted.md for the canonical +# strings. Prefix-only matching avoids coupling to the English suffix. +_REDACTED_PREFIX = "[REDACTED] " + + +def _is_redacted(value): + """True when the CLI returned an absent/empty value or a redaction sentinel.""" + if not value: + return True + return isinstance(value, str) and value.startswith(_REDACTED_PREFIX) + class SfdxOrgConfig(OrgConfig): """Org config which loads from sfdx keychain""" @@ -63,12 +76,19 @@ def sfdx_info(self): ) access_token = result.get("accessToken") - if not access_token: + fallback_fired = _is_redacted(access_token) + if fallback_fired: access_token = self._fetch_access_token(username) password = result.get("password") - if not password: + # Only call show-user-password when the CLI redacted the password + # field AND we already know we're on a new CLI (access-token fallback + # fired). Legacy-passwordless orgs leave `password` absent; calling + # show-user-password in that case is a pointless extra subprocess. + if fallback_fired and _is_redacted(password): password = self._fetch_user_password(username) + elif _is_redacted(password): + password = None sfdx_info = { "instance_url": result["instanceUrl"], @@ -181,7 +201,7 @@ def get_access_token(self, **userfields): else: username = result[0]["Username"] - p = sfdx(f"org display --target-org={username} --json") + p = sfdx(f"org display --target-org={shell_quote(username)} --json") if p.returncode: output = p.stdout_text.read() try: @@ -196,9 +216,9 @@ def get_access_token(self, **userfields): info = json.loads(p.stdout_text.read()) token = info.get("result", {}).get("accessToken") - if token: - return token - return self._fetch_access_token(username) + if _is_redacted(token): + return self._fetch_access_token(username) + return token def _fetch_access_token(self, username): """Retrieve an access token using `sf org auth show-access-token`. @@ -206,7 +226,8 @@ def _fetch_access_token(self, username): Used as a fallback when `sf org display` redacts the token (SF CLI 2.136+). """ p = sfdx( - f"org auth show-access-token --target-org={username} --json --no-prompt" + f"org auth show-access-token --target-org={shell_quote(username)}" + f" --json --no-prompt" ) output = p.stdout_text.read() if p.returncode: @@ -227,7 +248,9 @@ def _fetch_access_token(self, username): f"Failed to parse JSON from `sf org auth show-access-token`: {e}" ) token = info.get("result", {}).get("accessToken") - if not token: + if _is_redacted(token): + # Defensive: if show-access-token itself ever returns a sentinel, + # treat it as failure rather than handing back a placeholder. raise SfdxOrgException( f"Unable to retrieve access token for {username}. " f"Tried `sf org display` and `sf org auth show-access-token`. " @@ -242,7 +265,8 @@ def _fetch_user_password(self, username): Returns None if the org has no password or the command fails; never raises. """ p = sfdx( - f"org auth show-user-password --target-org={username} --json --no-prompt" + f"org auth show-user-password --target-org={shell_quote(username)}" + f" --json --no-prompt" ) if p.returncode: return None @@ -251,7 +275,9 @@ def _fetch_user_password(self, username): except JSONDecodeError: return None password = info.get("result", {}).get("password") - return password if password else None + if _is_redacted(password): + return None + return password def force_refresh_oauth_token(self): # Call org display and parse output to get instance_url and diff --git a/cumulusci/core/config/tests/test_config_expensive.py b/cumulusci/core/config/tests/test_config_expensive.py index b5275b8105..7843d662ef 100644 --- a/cumulusci/core/config/tests/test_config_expensive.py +++ b/cumulusci/core/config/tests/test_config_expensive.py @@ -669,9 +669,7 @@ def test_fetch_user_password_returns_none_on_empty(self, Command): def test_sfdx_info_sentinel_token_triggers_fallback(self, Command): """Spec round 3: redacted token is a truthy sentinel, not absent.""" - redacted_token = ( - "[REDACTED] Use 'sf org auth show-access-token' to view" - ) + redacted_token = "[REDACTED] Use 'sf org auth show-access-token' to view" org_display = ( b'{"result": {"id": "00DDP000006GRcP2AW", "instanceUrl": "url",' b' "username": "username", "accessToken":' @@ -679,6 +677,8 @@ def test_sfdx_info_sentinel_token_triggers_fallback(self, Command): b' "1970-01-01T00:00:00Z", "expirationDate": "1970-01-08"}}' ) token_fetch = b'{"result": {"accessToken": "real-token"}}' + # Password absent in org_display -> _is_redacted True -> show-user-password runs. + password_fetch = b'{"result": {"password": null}}' Command.side_effect = [ mock.Mock( @@ -687,6 +687,9 @@ def test_sfdx_info_sentinel_token_triggers_fallback(self, Command): mock.Mock( stderr=io.BytesIO(b""), stdout=io.BytesIO(token_fetch), returncode=0 ), + mock.Mock( + stderr=io.BytesIO(b""), stdout=io.BytesIO(password_fetch), returncode=0 + ), ] config = SfdxOrgConfig({"username": "test", "created": True}, "test") @@ -694,7 +697,7 @@ def test_sfdx_info_sentinel_token_triggers_fallback(self, Command): assert info["access_token"] == "real-token" assert not info["access_token"].startswith("[REDACTED]") - assert Command.call_count == 2 + assert Command.call_count == 3 def test_fetch_user_password_returns_none_on_sentinel(self, Command): """Spec round 3: show-user-password returning a sentinel = no password.""" @@ -714,9 +717,7 @@ def test_fetch_user_password_returns_none_on_sentinel(self, Command): def test_get_access_token_sentinel_triggers_fallback(self, Command): """Spec round 3: get_access_token must detect the sentinel, not just absence.""" - redacted_token = ( - "[REDACTED] Use 'sf org auth show-access-token' to view" - ) + redacted_token = "[REDACTED] Use 'sf org auth show-access-token' to view" sf = mock.Mock() sf.query_all.return_value = {"records": [{"Username": "whatever@example.com"}]} @@ -750,10 +751,12 @@ def test_fetch_access_token_shell_quotes_username(self, Command): ) config = SfdxOrgConfig({"username": "test"}, "test") - sfdx_mock = mock.Mock(return_value=mock.Mock( - returncode=0, - stdout_text=io.BytesIO(b'{"result": {"accessToken": "fetched-token"}}'), - )) + sfdx_mock = mock.Mock( + return_value=mock.Mock( + returncode=0, + stdout_text=io.BytesIO(b'{"result": {"accessToken": "fetched-token"}}'), + ) + ) # Build a payload that includes a stdout_text read() side-effect. sfdx_mock.return_value.stdout_text.read = mock.Mock( return_value=b'{"result": {"accessToken": "fetched-token"}}' @@ -764,13 +767,11 @@ def test_fetch_access_token_shell_quotes_username(self, Command): cmd = sfdx_mock.call_args_list[0][0][0] # Raw, unquoted username would let the semicolon break out of the arg. assert "weird;user@example.com" not in cmd or ( - "'weird;user@example.com'" in cmd - or '"weird;user@example.com"' in cmd + "'weird;user@example.com'" in cmd or '"weird;user@example.com"' in cmd ) # Stronger form: the quoted token must appear. assert ( - "'weird;user@example.com'" in cmd - or '"weird;user@example.com"' in cmd + "'weird;user@example.com'" in cmd or '"weird;user@example.com"' in cmd ), f"username not shell-quoted in command: {cmd!r}" def test_instance_url(self, Command): From 6171022e9be2dfb1a11f5613a6129d09c78f3278 Mon Sep 17 00:00:00 2001 From: James Estevez Date: Thu, 25 Jun 2026 16:46:45 -0700 Subject: [PATCH 09/13] fix(sfdx): gate password fallback on sentinel only, not on absence Round 3 review caught that the previous password gate fired one extra subprocess for the most common new-CLI org class. A new CLI returns the token sentinel + OMITS `password` entirely for a passwordless org (web-auth sandbox, scratch org without `force:user:password:generate`). The previous `fallback_fired AND _is_redacted(password)` gate evaluated True/True on every sfdx_info refresh and ran a pointless `sf org auth show-user-password`. Split the helper: - `_is_sentinel(v)` -- strict prefix check, used for the password gate. An absent password is not a sentinel. - `_is_redacted(v)` -- absent OR sentinel, used for the token path. An absent token is as actionable as a redacted one (both mean "ask `sf org auth show-access-token`"). Test coverage: - `test_sfdx_info_new_cli_fetches_token` corrected: assert call_count == 2 (was 3) and password absent. Codifies the passwordless-org-no-fallback contract. - `test_sfdx_info_sentinel_token_triggers_fallback` (round-2) corrected the same way. - `test_sfdx_info_new_cli_fetches_password` rewritten: both fields are now sentinels (the real shape for a scratch org with a generated password on a new CLI), not absence. - New `test_sfdx_info_new_cli_no_password_org`: explicit passwordless-org no-fallback assertion. - New `test_sfdx_info_sentinel_password_uses_fetched`: sfdx_info- level coverage of the both-sentinels path. Was helper-level only. Files reformatted with `uvx ruff format`. Pre-commit black 22.3.0 incompatible with local Python 3.14 (`ast.Str` removed in 3.12); `SKIP=black` follows the precedent set by 71b8e136e + bdc205671. 119/119 tests green in test_config_expensive + test_org. --- cumulusci/cli/tests/test_org.py | 4 +- cumulusci/core/config/sfdx_org_config.py | 30 ++-- .../config/tests/test_config_expensive.py | 132 +++++++++++++++--- 3 files changed, 131 insertions(+), 35 deletions(-) diff --git a/cumulusci/cli/tests/test_org.py b/cumulusci/cli/tests/test_org.py index 1a0cc1b8c8..2393c8a725 100644 --- a/cumulusci/cli/tests/test_org.py +++ b/cumulusci/cli/tests/test_org.py @@ -888,9 +888,9 @@ def get_org(orgname): run_click_command(org.org_list, runtime=runtime, json_flag=False, plain=False) - assert "Cannot load org config for `test1`" in str( + assert "Cannot load org config for `test1`" in str(echo.mock_calls), ( echo.mock_calls - ), echo.mock_calls + ) assert "NOPE!" in str(echo.mock_calls), echo.mock_calls assert "Cannot cleanup org cache dirs" in str(echo.mock_calls), echo.mock_calls diff --git a/cumulusci/core/config/sfdx_org_config.py b/cumulusci/core/config/sfdx_org_config.py index abcb056f1e..2e8a26ddd1 100644 --- a/cumulusci/core/config/sfdx_org_config.py +++ b/cumulusci/core/config/sfdx_org_config.py @@ -16,11 +16,22 @@ _REDACTED_PREFIX = "[REDACTED] " +def _is_sentinel(value): + """True only for the SF CLI 2.136+ redaction sentinel string.""" + return isinstance(value, str) and value.startswith(_REDACTED_PREFIX) + + def _is_redacted(value): - """True when the CLI returned an absent/empty value or a redaction sentinel.""" + """True when the CLI returned an absent/empty value or the redaction sentinel. + + Used for the access-token path: an absent token is as actionable as a redacted + one (both mean "ask `sf org auth show-access-token`"). For the password path, + prefer _is_sentinel so a legitimately-absent password (passwordless org on a + new CLI) does not trigger a pointless `sf org auth show-user-password` call. + """ if not value: return True - return isinstance(value, str) and value.startswith(_REDACTED_PREFIX) + return _is_sentinel(value) class SfdxOrgConfig(OrgConfig): @@ -76,18 +87,17 @@ def sfdx_info(self): ) access_token = result.get("accessToken") - fallback_fired = _is_redacted(access_token) - if fallback_fired: + if _is_redacted(access_token): access_token = self._fetch_access_token(username) password = result.get("password") - # Only call show-user-password when the CLI redacted the password - # field AND we already know we're on a new CLI (access-token fallback - # fired). Legacy-passwordless orgs leave `password` absent; calling - # show-user-password in that case is a pointless extra subprocess. - if fallback_fired and _is_redacted(password): + # Gate the password fallback on the sentinel ONLY. An absent password + # means the org has no password (web-auth sandbox, scratch org without + # `force:user:password:generate`); calling `sf org auth show-user-password` + # in that case is a pointless extra subprocess on every refresh. + if _is_sentinel(password): password = self._fetch_user_password(username) - elif _is_redacted(password): + elif not password: password = None sfdx_info = { diff --git a/cumulusci/core/config/tests/test_config_expensive.py b/cumulusci/core/config/tests/test_config_expensive.py index 7843d662ef..75d5382e36 100644 --- a/cumulusci/core/config/tests/test_config_expensive.py +++ b/cumulusci/core/config/tests/test_config_expensive.py @@ -311,7 +311,7 @@ def test_scratch_info(self, Command): assert config._sfdx_info_date def test_sfdx_info_new_cli_fetches_token(self, Command): - # First subprocess: sf org display redacts accessToken + # First subprocess: sf org display redacts accessToken; org has no password org_display = b"""{ "result": { "id": "00DDP000006GRcP2AW", @@ -323,8 +323,6 @@ def test_sfdx_info_new_cli_fetches_token(self, Command): }""" # Second subprocess: sf org auth show-access-token returns the token token_fetch = b'{"result": {"accessToken": "fetched-token"}}' - # Third subprocess: sf org auth show-user-password returns null (no password) - password_fetch = b'{"result": {"password": null}}' Command.side_effect = [ mock.Mock( @@ -333,8 +331,44 @@ def test_sfdx_info_new_cli_fetches_token(self, Command): mock.Mock( stderr=io.BytesIO(b""), stdout=io.BytesIO(token_fetch), returncode=0 ), + ] + + config = SfdxOrgConfig({"username": "test", "created": True}, "test") + info = config.sfdx_info + + assert info["access_token"] == "fetched-token" + assert info["org_id"] == "00DDP000006GRcP2AW" + assert "password" not in info + # Critical: no show-user-password call for a passwordless org. The + # password gate keys on the sentinel, not on absence. + assert Command.call_count == 2 + + def test_sfdx_info_new_cli_no_password_org(self, Command): + """New CLI + passwordless org: token sentinel, password absent. + + Must NOT call `sf org auth show-user-password` (the absent password is + not a sentinel; the org genuinely has no password). + """ + sentinel = ( + "[REDACTED] Use 'sf org auth show-access-token' " + "--target-org to view" + ) + org_display = ( + '{"result": {' + '"id": "00DDP000006GRcP2AW",' + '"instanceUrl": "url",' + f'"accessToken": "{sentinel}",' + '"username": "username"' + "}}" + ).encode() + token_fetch = b'{"result": {"accessToken": "fetched-token"}}' + + Command.side_effect = [ mock.Mock( - stderr=io.BytesIO(b""), stdout=io.BytesIO(password_fetch), returncode=0 + stderr=io.BytesIO(b""), stdout=io.BytesIO(org_display), returncode=0 + ), + mock.Mock( + stderr=io.BytesIO(b""), stdout=io.BytesIO(token_fetch), returncode=0 ), ] @@ -342,20 +376,76 @@ def test_sfdx_info_new_cli_fetches_token(self, Command): info = config.sfdx_info assert info["access_token"] == "fetched-token" - assert info["org_id"] == "00DDP000006GRcP2AW" assert "password" not in info + assert Command.call_count == 2 + + def test_sfdx_info_sentinel_password_uses_fetched(self, Command): + """New CLI + scratch org with generated password: BOTH fields are sentinels. + + Must call `sf org auth show-user-password` and surface the fetched value. + """ + token_sentinel = ( + "[REDACTED] Use 'sf org auth show-access-token' " + "--target-org to view" + ) + pw_sentinel = ( + "[REDACTED] Use 'sf org auth show-user-password' " + "--target-org to view" + ) + org_display = ( + '{"result": {' + '"id": "00DDP000006GRcP2AW",' + '"instanceUrl": "url",' + f'"accessToken": "{token_sentinel}",' + f'"password": "{pw_sentinel}",' + '"username": "username"' + "}}" + ).encode() + token_fetch = b'{"result": {"accessToken": "fetched-token"}}' + password_fetch = b'{"result": {"password": "fetched-pw"}}' + + Command.side_effect = [ + mock.Mock( + stderr=io.BytesIO(b""), stdout=io.BytesIO(org_display), returncode=0 + ), + mock.Mock( + stderr=io.BytesIO(b""), stdout=io.BytesIO(token_fetch), returncode=0 + ), + mock.Mock( + stderr=io.BytesIO(b""), stdout=io.BytesIO(password_fetch), returncode=0 + ), + ] + + config = SfdxOrgConfig({"username": "test", "created": True}, "test") + info = config.sfdx_info + + assert info["access_token"] == "fetched-token" + assert info["password"] == "fetched-pw" assert Command.call_count == 3 def test_sfdx_info_new_cli_fetches_password(self, Command): - org_display = b"""{ - "result": { - "id": "00DDP000006GRcP2AW", - "instanceUrl": "url", - "username": "username", - "createdDate": "1970-01-01T00:00:00Z", - "expirationDate": "1970-01-08" - } -}""" + # New CLI redacts BOTH fields when the org has a generated password + # (e.g. a scratch org). Password fallback fires only on the sentinel, + # not on absence. + token_sentinel = ( + "[REDACTED] Use 'sf org auth show-access-token' " + "--target-org to view" + ) + pw_sentinel = ( + "[REDACTED] Use 'sf org auth show-user-password' " + "--target-org to view" + ) + org_display = ( + '{"result": {' + '"id": "00DDP000006GRcP2AW",' + '"instanceUrl": "url",' + '"username": "username",' + f'"accessToken": "{token_sentinel}",' + f'"password": "{pw_sentinel}",' + '"createdDate": "1970-01-01T00:00:00Z",' + '"expirationDate": "1970-01-08"' + "}}" + ).encode() token_fetch = b'{"result": {"accessToken": "fetched-token"}}' password_fetch = b'{"result": {"password": "fetched-pw"}}' @@ -677,8 +767,6 @@ def test_sfdx_info_sentinel_token_triggers_fallback(self, Command): b' "1970-01-01T00:00:00Z", "expirationDate": "1970-01-08"}}' ) token_fetch = b'{"result": {"accessToken": "real-token"}}' - # Password absent in org_display -> _is_redacted True -> show-user-password runs. - password_fetch = b'{"result": {"password": null}}' Command.side_effect = [ mock.Mock( @@ -687,9 +775,6 @@ def test_sfdx_info_sentinel_token_triggers_fallback(self, Command): mock.Mock( stderr=io.BytesIO(b""), stdout=io.BytesIO(token_fetch), returncode=0 ), - mock.Mock( - stderr=io.BytesIO(b""), stdout=io.BytesIO(password_fetch), returncode=0 - ), ] config = SfdxOrgConfig({"username": "test", "created": True}, "test") @@ -697,7 +782,8 @@ def test_sfdx_info_sentinel_token_triggers_fallback(self, Command): assert info["access_token"] == "real-token" assert not info["access_token"].startswith("[REDACTED]") - assert Command.call_count == 3 + # Password absent in org_display -> NOT a sentinel -> no fallback subprocess. + assert Command.call_count == 2 def test_fetch_user_password_returns_none_on_sentinel(self, Command): """Spec round 3: show-user-password returning a sentinel = no password.""" @@ -770,9 +856,9 @@ def test_fetch_access_token_shell_quotes_username(self, Command): "'weird;user@example.com'" in cmd or '"weird;user@example.com"' in cmd ) # Stronger form: the quoted token must appear. - assert ( - "'weird;user@example.com'" in cmd or '"weird;user@example.com"' in cmd - ), f"username not shell-quoted in command: {cmd!r}" + assert "'weird;user@example.com'" in cmd or '"weird;user@example.com"' in cmd, ( + f"username not shell-quoted in command: {cmd!r}" + ) def test_instance_url(self, Command): config = ScratchOrgConfig({}, "test") From 17120e0c63cbaa7b4699914448ddd062c5cb102b Mon Sep 17 00:00:00 2001 From: James Estevez Date: Thu, 25 Jun 2026 17:39:21 -0700 Subject: [PATCH 10/13] style(sfdx): polish round-4 nits Address round-4 review comments (no blocking findings; nit polish only). - Add load-bearing trailing-space comment to `_REDACTED_PREFIX`. The CLI sentinel is the literal `"[REDACTED] Use ..."` form; a prefix without the trailing space would also match the unrelated string `"[REDACTED]Use ..."` if Salesforce ever shipped that. - Tighten `_fetch_user_password` docstring: name every failure path (non-zero exit, malformed JSON, sentinel result) and the policy (return None, never raise; debug-log to keep the call diagnosable). - Add debug logs on both `_fetch_user_password` failure branches so transient subprocess / JSON failures surface in `cci --debug` runs without promoting the call to raise. - Drop the `elif not password: password = None` no-op. `result.get()` already returns None for absent keys; the conditional was vestigial from the compound-gate version. - Drop the redundant first assertion in `test_fetch_access_token_shell_quotes_username`. The second assertion is strictly stronger. - Add `Command.call_count == 1` to `test_scratch_info` to codify "legacy CLI: no fallback subprocesses fire" at the test level. - Add `Command.call_count == 3` to `test_sfdx_info_new_cli_fetches_password` to codify the both- sentinels path (org_display + show-access-token + show-user-password). 77/77 tests green in test_config_expensive. `uvx ruff check` and `uvx ruff format --check` clean. SKIP=black per branch precedent (71b8e136e -> bdc205671 -> 7957c49b3); black 22.3.0 incompatible with local Python 3.14. --- cumulusci/core/config/sfdx_org_config.py | 21 ++++++++++++++----- .../config/tests/test_config_expensive.py | 12 ++++++----- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/cumulusci/core/config/sfdx_org_config.py b/cumulusci/core/config/sfdx_org_config.py index 2e8a26ddd1..b8db05d043 100644 --- a/cumulusci/core/config/sfdx_org_config.py +++ b/cumulusci/core/config/sfdx_org_config.py @@ -13,6 +13,8 @@ # credential fields when SF_TEMP_SHOW_SECRETS is unset. See # @salesforce/plugin-org messages/secrets-redacted.md for the canonical # strings. Prefix-only matching avoids coupling to the English suffix. +# The trailing space is load-bearing: the CLI sentinel is the literal +# "[REDACTED] Use ..." form, never "[REDACTED]Use ...". _REDACTED_PREFIX = "[REDACTED] " @@ -97,8 +99,6 @@ def sfdx_info(self): # in that case is a pointless extra subprocess on every refresh. if _is_sentinel(password): password = self._fetch_user_password(username) - elif not password: - password = None sfdx_info = { "instance_url": result["instanceUrl"], @@ -271,18 +271,29 @@ def _fetch_access_token(self, username): def _fetch_user_password(self, username): """Retrieve the org password using `sf org auth show-user-password`. - Used as a fallback when `sf org display` redacts the password (SF CLI 2.136+). - Returns None if the org has no password or the command fails; never raises. + Used as a fallback when `sf org display` redacts the password (SF CLI + 2.136+). Returns None on any failure path (non-zero exit, malformed + JSON, sentinel result) since the password is optional. Failures are + logged at debug level so transient issues can be diagnosed without + promoting the call to raise. """ p = sfdx( f"org auth show-user-password --target-org={shell_quote(username)}" f" --json --no-prompt" ) if p.returncode: + self.logger.debug( + f"sf org auth show-user-password exited {p.returncode} for " + f"{username}; treating as no password." + ) return None try: info = json.loads(p.stdout_text.read()) - except JSONDecodeError: + except JSONDecodeError as e: + self.logger.debug( + f"Failed to parse JSON from sf org auth show-user-password " + f"for {username}: {e}. Treating as no password." + ) return None password = info.get("result", {}).get("password") if _is_redacted(password): diff --git a/cumulusci/core/config/tests/test_config_expensive.py b/cumulusci/core/config/tests/test_config_expensive.py index 75d5382e36..1ed24c74a2 100644 --- a/cumulusci/core/config/tests/test_config_expensive.py +++ b/cumulusci/core/config/tests/test_config_expensive.py @@ -309,6 +309,9 @@ def test_scratch_info(self, Command): ): assert key in config.config assert config._sfdx_info_date + # Legacy CLI: a single org_display call must satisfy sfdx_info; no + # show-access-token / show-user-password fallback subprocesses. + assert Command.call_count == 1 def test_sfdx_info_new_cli_fetches_token(self, Command): # First subprocess: sf org display redacts accessToken; org has no password @@ -466,6 +469,8 @@ def test_sfdx_info_new_cli_fetches_password(self, Command): assert info["access_token"] == "fetched-token" assert info["password"] == "fetched-pw" + # Both sentinels present: org_display + show-access-token + show-user-password. + assert Command.call_count == 3 def test_sfdx_info_token_fetch_fails(self, Command): org_display = b"""{ @@ -851,11 +856,8 @@ def test_fetch_access_token_shell_quotes_username(self, Command): config._fetch_access_token("weird;user@example.com") cmd = sfdx_mock.call_args_list[0][0][0] - # Raw, unquoted username would let the semicolon break out of the arg. - assert "weird;user@example.com" not in cmd or ( - "'weird;user@example.com'" in cmd or '"weird;user@example.com"' in cmd - ) - # Stronger form: the quoted token must appear. + # The username must appear shell-quoted; raw interpolation under + # shell=True would let the semicolon break out of the arg. assert "'weird;user@example.com'" in cmd or '"weird;user@example.com"' in cmd, ( f"username not shell-quoted in command: {cmd!r}" ) From 6deb48ed8f0399cfc5c0419ae62dbc3c472cf69e Mon Sep 17 00:00:00 2001 From: James Estevez Date: Thu, 25 Jun 2026 17:44:10 -0700 Subject: [PATCH 11/13] docs(sfdx): note locale fragility of `[REDACTED] ` prefix Reviewer suggestion. Flag the only quiet failure mode of the sentinel-prefix detection in `_REDACTED_PREFIX`'s docstring: `@salesforce/core` currently hardcodes locale to en_US (per-locale message bundles are an unimplemented TODO upstream), so the "[REDACTED] " literal is stable on every shipping CLI. The literal lives inside the translatable message string, however. If Salesforce later wires up localization, a non-English `redacted.accessToken` could drop the prefix and silently break detection. Note the more robust long-term signal (match sentinel message keys, or "value is not a well-formed access token") near the constant so a future reader sees the upgrade path without spelunking the CLI repo. Comment-only; no behavioral change. 77/77 tests still green. SKIP=black per branch precedent. --- cumulusci/core/config/sfdx_org_config.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/cumulusci/core/config/sfdx_org_config.py b/cumulusci/core/config/sfdx_org_config.py index b8db05d043..df8b274491 100644 --- a/cumulusci/core/config/sfdx_org_config.py +++ b/cumulusci/core/config/sfdx_org_config.py @@ -15,6 +15,16 @@ # strings. Prefix-only matching avoids coupling to the English suffix. # The trailing space is load-bearing: the CLI sentinel is the literal # "[REDACTED] Use ..." form, never "[REDACTED]Use ...". +# +# Locale fragility: today `@salesforce/core`'s Messages.getLocale() is +# hardcoded to en_US (per-locale message bundles are an unimplemented +# TODO upstream), so this prefix is stable on every shipping CLI. The +# "[REDACTED] " literal lives inside the translatable message string, +# however; if Salesforce later wires up localization, a non-English +# `redacted.accessToken` could drop this prefix and silently break +# detection. A locale-independent signal (matching the three sentinel +# message keys, or "value is not a well-formed access token") would be +# more robust if/when that lands. _REDACTED_PREFIX = "[REDACTED] " From 928a3f6f967c14e7dbdab0532b4e8804cabff842 Mon Sep 17 00:00:00 2001 From: James Estevez Date: Thu, 25 Jun 2026 17:47:32 -0700 Subject: [PATCH 12/13] docs(sfdx): add tripwire to `_REDACTED_PREFIX` locale-fragility note Reviewer add-on. The locale-fragility comment names the failure mode but leaves the on-call engineer to infer the symptom. Add one tripwire sentence: if `sf org display --json`'s access token ever round-trips into an API 401 with a human-readable token value, suspect this detection path first. Converts the comment from "here is the theoretical risk" to "here is the symptom that means this assumption broke." Comment-only; no behavioral change. --- cumulusci/core/config/sfdx_org_config.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cumulusci/core/config/sfdx_org_config.py b/cumulusci/core/config/sfdx_org_config.py index df8b274491..042b4c6436 100644 --- a/cumulusci/core/config/sfdx_org_config.py +++ b/cumulusci/core/config/sfdx_org_config.py @@ -25,6 +25,9 @@ # detection. A locale-independent signal (matching the three sentinel # message keys, or "value is not a well-formed access token") would be # more robust if/when that lands. +# +# Tripwire: if a `sf org display --json` access token round-trips into +# an API 401 with a human-readable token value, suspect this path first. _REDACTED_PREFIX = "[REDACTED] " From d2b1471364c77e90e6a7caafc97f28b7eec266ed Mon Sep 17 00:00:00 2001 From: brahmsagargaur-gaur Date: Tue, 8 Sep 2026 12:21:00 +0530 Subject: [PATCH 13/13] docs(contributing): update local setup to uv, add UTC test note MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rewrite the contributor setup steps to use uv instead of the venv + make dev-install + pip flow: - Install uv (curl installer, with brew/pip alternatives noted) - Replace `python3 -m venv` / `make dev-install` with `uv sync --all-groups`, which provisions a compatible Python (3.11–3.13), creates `.venv`, and installs CumulusCI in editable mode - Prefix test/lint commands with `uv run` (pre-commit, pytest, coverage) and note that activating `.venv` lets you drop the prefix - Add `isort` to the listed pre-commit hooks Also add a note under "Testing CumulusCI" that some tests assume the system clock is UTC (they derive values from `datetime.utcnow()`), so tests like `test_days_alive`, `test_format_days`, and `test_org_list` can fail with off-by-one date diffs in other time zones. Suggest prefixing with `TZ=UTC`. --- docs/contributing.md | 40 ++++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/docs/contributing.md b/docs/contributing.md index 69866ec3c0..84530174eb 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -56,30 +56,38 @@ development. 2. Clone your fork to your local workspace. -3. Create a fresh Python 3 virtual environment and activate it (to keep - this isolated from other Python software on your machine). Here is - one way: +3. Install [uv](https://docs.astral.sh/uv/), which CumulusCI uses to + manage its virtual environment and dependencies. If you don't already + have it: - $ python3 -m venv cci_venv - $ source cci_venv/bin/activate + $ curl -LsSf https://astral.sh/uv/install.sh | sh -4. Install the development requirements: + (`brew install uv` and `pip install uv` also work.) - $ make dev-install +4. From the repository root, create the environment and install + CumulusCI with all of its dependencies. `uv sync` provisions a + compatible Python (CumulusCI requires Python 3.11–3.13), creates a + `.venv`, and installs CumulusCI in editable mode. `--all-groups` + pulls in the `dev` and `lint` dependencies you'll need below: -5. Install `pre-commit` hooks for `black` and `flake8`: + $ uv sync --all-groups - $ pre-commit install --install-hooks +5. Install `pre-commit` hooks for `black`, `flake8`, and `isort`: -6. After making changes, run the tests and make sure they all pass: + $ uv run pre-commit install --install-hooks - $ pytest +6. After making changes, run the tests and make sure they all pass. + Prefixing a command with `uv run` executes it inside the project + environment (alternatively, activate it once with + `source .venv/bin/activate` and drop the prefix): + + $ uv run pytest 7. Your new code should also have meaningful tests. One way to double check that your tests cover everything is to ensure that your new code has test code coverage: - $ make coverage + $ uv run make coverage 8. Push your changes to GitHub and submit a Pull Request. The base branch should be a new feature branch that we create to receive the @@ -112,6 +120,14 @@ Before you submit a pull request, check that it meets these guidelines: ## Testing CumulusCI +```{note} +Some tests assume the system clock is set to UTC and will fail in other +time zones, because they derive values such as scratch-org age from +`datetime.utcnow()`. If tests like `test_days_alive`, `test_format_days`, +or `test_org_list` fail with off-by-one date differences, prefix your +command with `TZ=UTC`, e.g. `TZ=UTC uv run pytest`. +``` + ### Org-reliant Automated Tests Some tests are marked `@pytest.mark.vcr()` which means that they can