From df129652e6b9bb79b324643b7782d64b54502fcb Mon Sep 17 00:00:00 2001 From: Daniel Petty Date: Thu, 5 Nov 2026 17:19:14 +0000 Subject: [PATCH 1/3] fix(security): mask the user name of a URL with no password redact_text and mask_uri_credentials masked the userinfo of a URL only when it held a password. A token used as the user name, with an empty password, showed in masked text. httpx2 sends such a URL as Basic authentication. The user name is now masked, whatever the scheme: in redact_text and *** in mask_uri_credentials. A parser cannot tell a token from a name such as git, so the rule masks both, as pip does. The module docstring of the sanitizer states the rule and the tests hold it. Plans #2662. --- .../.openspec.yaml | 2 + .../design.md | 51 +++++++++++++++ .../proposal.md | 48 ++++++++++++++ .../specs/secret-masking/spec.md | 46 +++++++++++++ .../tasks.md | 23 +++++++ src/devops_cli/security/sanitizer.py | 55 +++++++++++++--- .../ai/client/test_url_credential_masking.py | 23 +++++-- tests/k8s/test_gitea_doctor.py | 4 +- tests/security/test_redaction_properties.py | 22 +++++++ tests/security/test_secret_redaction.py | 65 +++++++++++++++++-- 10 files changed, 320 insertions(+), 19 deletions(-) create mode 100644 openspec/changes/2662-mask-url-user-name-without-password/.openspec.yaml create mode 100644 openspec/changes/2662-mask-url-user-name-without-password/design.md create mode 100644 openspec/changes/2662-mask-url-user-name-without-password/proposal.md create mode 100644 openspec/changes/2662-mask-url-user-name-without-password/specs/secret-masking/spec.md create mode 100644 openspec/changes/2662-mask-url-user-name-without-password/tasks.md diff --git a/openspec/changes/2662-mask-url-user-name-without-password/.openspec.yaml b/openspec/changes/2662-mask-url-user-name-without-password/.openspec.yaml new file mode 100644 index 000000000..1491b921e --- /dev/null +++ b/openspec/changes/2662-mask-url-user-name-without-password/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-11-05 diff --git a/openspec/changes/2662-mask-url-user-name-without-password/design.md b/openspec/changes/2662-mask-url-user-name-without-password/design.md new file mode 100644 index 000000000..c7d8cca32 --- /dev/null +++ b/openspec/changes/2662-mask-url-user-name-without-password/design.md @@ -0,0 +1,51 @@ +# Design + +## Context + +See `proposal.md`. `_mask_url_candidate` in `src/devops_cli/security/sanitizer.py` handles each URL that `redact_text` finds in free text. `mask_uri_credentials` handles one URL. Both return a URL with no password as it was. + +## Goals / Non-Goals + +**Goals:** + +- A token in the user name of a URL does not show in masked text. +- The rule is written in the docs of the sanitizer and held by tests. + +**Non-Goals:** + +- A user name that holds a `/`, `?` or `#` that is not percent-encoded. It is not valid userinfo. The parser reads the part after the character as the host or the path, so no rule can mask it. +- A URL with no `://`, such as the scp form `git@host:org/repo.git`. `redact_text` finds a URL by its `://` and has never changed such a text. +- The keyword rules for `token` and `password`. Issue #2613 owns them, and its pull request is merged. + +## Decisions + +### 1. Mask a user name with no password + +The user name is masked in `redact_text` and in `mask_uri_credentials`, in every scheme. + +- *Rationale*: a service can take a token as the user name. httpx2 sends the URL as Basic authentication, so the user name is a credential. A parser cannot tell a token from a harmless name. The safe rule masks both. +- *Rationale*: pip masks a user name with no password in the same way. +- *Rejected alternative*: mask only a user name that looks like a token, with `_looks_like_a_credential`. A token with no digit, no capital and fewer than 24 characters would show. A token can be a plain word, and a name such as `git` is also a plain word, so the check cannot split them. +- *Rejected alternative*: keep a list of harmless names such as `git`. The list grows with each service, and a token that matches an entry would show. +- *Cost*: `ssh://git@host/path` shows as `ssh://@host/path` in masked output. The host and the path still show, so the text keeps its use. The message of the `clone-url` check of the Gitea doctor (`devops k8s doctor`) for a wrong `ssh_url` is the one such text in the tests, and its test now expects ``. + +### 2. Use the parser that each function already uses + +`_mask_url_candidate` reads `urlsplit` and `mask_uri_credentials` reads `httpx2.URL`. Each already finds the userinfo. The change adds one branch to each: a user name with no password. + +- *Rejected alternative*: a regex for `scheme://user@`. The issue and `AGENTS.md` ask for a standard parser. A regex fails on IPv6 hosts, on `@` in a query and on a password with `@`, which #1109 fixed. + +### 3. Keep the shape of the old placeholders + +`redact_text` shows `` for a user name alone, as it shows `:` for a user and a password. `mask_uri_credentials` shows `***` for a user name alone, as it shows `***` for a password with no user. A user name with a password keeps the name and shows `user:***`, so the three forms of one function stay consistent. + +- *Rejected alternative*: a new placeholder such as ``. The sanitizer does not know that the name is a token. + +### 4. An empty user name stays + +`http://@example.com/x` has an empty user name and no password. It holds no credential, so both functions return it as it was. The form `http://:@example.com/` also stays in `mask_uri_credentials`. + +## Risks / Trade-offs + +- [Risk] A harmless user name in a clone URL is masked, so a person reads `ssh://@host/path` and not the name. + - Mitigation: the host and the path stay. A person who needs the name reads the config or the remote, which the masker does not touch. The choice is one place in the module docstring, so a person can reverse it. diff --git a/openspec/changes/2662-mask-url-user-name-without-password/proposal.md b/openspec/changes/2662-mask-url-user-name-without-password/proposal.md new file mode 100644 index 000000000..84af0bc3f --- /dev/null +++ b/openspec/changes/2662-mask-url-user-name-without-password/proposal.md @@ -0,0 +1,48 @@ +# Proposal + +## Why + +Issue #2662: `redact_text` and `mask_uri_credentials` in `src/devops_cli/security/sanitizer.py` leave the user name of a URL as written when the URL has no password. + +- `_mask_url_candidate` masks the userinfo only when `parts.password is not None`. `mask_uri_credentials` masks it only when `u.password` is set. A URL such as `http://@example.com:11434/api/embed` has a user name and no password, so both functions return it unchanged. +- Some services take a token as the user name, with an empty password. httpx2 sends such a URL as Basic authentication, and a token can be in `ai.ollama_urls`. An error text that quotes the URL then shows the token. The same text with a user name and a password shows `:`. The two cases differ. +- A parser cannot tell a token from a harmless name such as `git` in `ssh://git@host/`. The issue asks for a rule. The rule goes in the docs of the sanitizer and in its tests. + +## What Changes + +- The rule: the user name of a URL is masked when the URL has no password. The scheme does not matter. A harmless name is masked too, because a parser cannot tell it from a token. +- `redact_text` shows `` in place of the user name. The host, the port, the path, the query and the fragment stay as written. +- `mask_uri_credentials` shows `***` in place of the user name. A user name with a password keeps its name and shows `user:***`, as before. +- The module docstring of `sanitizer.py` states the rule, and the docstrings of `redact_text` and `mask_uri_credentials` point to it. +- The output of `redact_text`, `mask_secrets` and `mask_uri_credentials` is a result that a consumer sees. So the change carries a spec delta. It adds three requirements to the capability `secret-masking`: the user name with no password is masked, a URL with no user name stays as written, and a user name with a password keeps its masking. +- The tests that held the old behavior now hold the new rule: the cases `custom://myuser@example.com/path` and `https://ghp_x@example.com/o/r` in `tests/ai/client/test_url_credential_masking.py`, the case `user-only` in `tests/security/test_secret_redaction.py`, and the message of the case `ssh-url-form-names-the-tunnel-host` in `tests/k8s/test_gitea_doctor.py`, which quotes `ssh://git@:2222/...`. +- This change fits one pull request. + +## Capabilities + +### New Capabilities + +None. + +### Modified Capabilities + +- `secret-masking`: the user name of a URL with no password is masked. + +## Impact + +- `src/devops_cli/security/sanitizer.py`. +- `tests/security/test_secret_redaction.py`, `tests/security/test_redaction_properties.py`, `tests/ai/client/test_url_credential_masking.py` and `tests/k8s/test_gitea_doctor.py`. +- A text with `ssh://git@host/path` now shows `ssh://@host/path` in masked output, for example in the message of the Gitea check of `devops k8s doctor` for a wrong `ssh_url`. This is the cost of the rule, and the design records it. + +## Feasibility + +- On `origin/main` at `f6d57ecd`, `redact_text("error calling http://tok_0123456789abcdef@example.com:11434/api/embed failed")` returned the text as it was. The same text with `user:pw12345` as the userinfo returned `:`. `mask_uri_credentials("http://tok_0123456789abcdef@example.com:11434/api/embed")` returned the URL as it was. The premise of the issue holds. +- httpx2 2.13.1 sends such a URL as Basic authentication. `httpx2.Client().build_request` on `http://tok_0123456789abcdef@example.com:11434/api/embed` gives a `BasicAuth` flow, and the request carries an `authorization` header. So a token in the user name is a real credential. +- `urllib.parse.urlsplit` gives `username='tok_0123456789abcdef'` and `password=None` for the same URL. `httpx2.URL` gives the user name and an empty password. Both parsers separate the userinfo from the host, so the change needs no pattern of its own. The IPv6 host `[2001:db8::1]:11434` and an `@` in a query stay correct, and the tests cover both. +- pip 26.3 masks a user name with no password. `redact_auth_from_url("http://tok_0123456789abcdef@example.com/x")` returns `http://****@example.com/x`, and the form with a password returns `http://user:****@example.com/x`. So the rule follows an open, maintained tool. +- Issue #1109 (closed) made the parsers `urllib` and httpx2. Pull request #3639 (issue #2613) changed `sanitizer.py` and is merged. It changed the keyword rules and not the URL step, so its behavior stays: prose after a token colon is unmasked. No open pull request touches `sanitizer.py`. +- The refine step of the issue answered the open question: mask the user name. The reason is least exposure, as a token and a name look the same. A person can reverse the choice, and the module docstring and the tests are the one place to change. + +## Existing Tool Considered + +The alternatives check of the issue (2026-11-04) found no tool that scans free text and masks a user name with a placeholder. The change uses the two parsers that the module already uses, `urllib.parse.urlsplit` and `httpx2.URL`. It adds no pattern and no helper. pip uses the same rule for its own output, as the Feasibility section shows. The change deletes no internal code. diff --git a/openspec/changes/2662-mask-url-user-name-without-password/specs/secret-masking/spec.md b/openspec/changes/2662-mask-url-user-name-without-password/specs/secret-masking/spec.md new file mode 100644 index 000000000..7220265b2 --- /dev/null +++ b/openspec/changes/2662-mask-url-user-name-without-password/specs/secret-masking/spec.md @@ -0,0 +1,46 @@ +## ADDED Requirements + +### Requirement: The user name of a URL is masked when the URL has no password +The secret masking system SHALL mask the user name of a URL that has a user name and no password, whatever the scheme. `redact_text` and `mask_secrets` SHALL show `` in place of the user name, in data and code syntax. `mask_uri_credentials` SHALL show `***`. The scheme, the host, the port, the path, the query and the fragment SHALL stay as written. + +#### Scenario: A token used as the user name in a text +- **WHEN** secret masking processes `error calling http://tok_0123456789abcdef@example.com:11434/api/embed failed` +- **THEN** the result is `error calling http://@example.com:11434/api/embed failed` + +#### Scenario: A harmless user name +- **WHEN** secret masking processes `ssh://git@example.com/org/repo.git` +- **THEN** the result is `ssh://@example.com/org/repo.git` + +#### Scenario: A user name in a URL with an IPv6 host, a query and a fragment +- **WHEN** secret masking processes `http://tok123@[2001:db8::1]:11434/x?a=b#c` +- **THEN** the result is `http://@[2001:db8::1]:11434/x?a=b#c` + +#### Scenario: One URI with a user name and no password +- **WHEN** `mask_uri_credentials` processes `https://tok_0123456789abcdef@example.com:11434/api/embed` +- **THEN** the result is `https://***@example.com:11434/api/embed` + +### Requirement: A URL with no user name stays as written +The secret masking system SHALL leave a URL as written when it has no userinfo, when its user name is empty and it has no password, or when its only `@` is after the authority. + +#### Scenario: An at sign in a query +- **WHEN** secret masking processes `http://example.com:11434/p?email=a@example.com` +- **THEN** the text comes back unchanged + +#### Scenario: An empty user name +- **WHEN** secret masking processes `http://@example.com/x` +- **THEN** the text comes back unchanged + +### Requirement: A URL with a user name and a password keeps its masking +The secret masking system SHALL mask both the user name and the password of a URL that has both. `redact_text` SHALL show `:`. `mask_uri_credentials` SHALL keep the user name and show `user:***`. A password with no user name SHALL become `***` in `mask_uri_credentials`. + +#### Scenario: A user name and a password in a text +- **WHEN** secret masking processes `http://user:pw12345@example.com/x` +- **THEN** the result is `http://:@example.com/x` + +#### Scenario: One URI with a user name and a password +- **WHEN** `mask_uri_credentials` processes `https://user:secret123@example.com:8443/api` +- **THEN** the result is `https://user:***@example.com:8443/api` + +#### Scenario: One URI with a password and no user name +- **WHEN** `mask_uri_credentials` processes `http://:supersecretpass@example.com:8080/data` +- **THEN** the result is `http://***@example.com:8080/data` diff --git a/openspec/changes/2662-mask-url-user-name-without-password/tasks.md b/openspec/changes/2662-mask-url-user-name-without-password/tasks.md new file mode 100644 index 000000000..2545b90e0 --- /dev/null +++ b/openspec/changes/2662-mask-url-user-name-without-password/tasks.md @@ -0,0 +1,23 @@ +# Tasks + +## 1. The user name of a URL with no password is masked + +Kind: behavior. Reads: `src/devops_cli/security/sanitizer.py`, `tests/security/test_secret_redaction.py`. Changes: `src/devops_cli/security/sanitizer.py`, `tests/security/test_secret_redaction.py`, `tests/security/test_redaction_properties.py`, `tests/ai/client/test_url_credential_masking.py`, `tests/k8s/test_gitea_doctor.py`. + +- [x] 1.1 In `tests/security/test_secret_redaction.py`, add a test that `redact_text` masks the user name of a URL with no password, in data and code syntax. Cover a token, a harmless name, JSON text, a URL inside a query, a user and a password, an `@` after the authority and an empty user name. Change the `user-only` case of the `mask_uri_credentials` test to the new rule. In `tests/security/test_redaction_properties.py`, add a property for any scheme and host. In `tests/ai/client/test_url_credential_masking.py`, change the cases that held the old behavior and add an IPv6 case. In `tests/k8s/test_gitea_doctor.py`, change the expected message of the `ssh://git@` case to show `` + - Verification: `uv run devops ci test tests/security/test_secret_redaction.py tests/security/test_redaction_properties.py tests/ai/client/test_url_credential_masking.py` fails on the base +- [x] 1.2 In `_mask_url_candidate`, mask a user name with no password as ``. In `mask_uri_credentials`, mask a user name with no password as `***`. Use the parser results that the functions read today, and add no pattern + - Verification: `uv run devops ci test tests/security tests/ai/client/test_url_credential_masking.py tests/invariants/test_architectural_invariants.py` +- [x] 1.3 State the rule in the module docstring of `sanitizer.py`, and point to it from the docstrings of `redact_text` and `mask_uri_credentials` + - Verification: `git grep -n "masked-user" src/devops_cli/security/sanitizer.py` + +## 2. Archive + +Kind: documentation. Reads: none. Changes: `openspec/changes/`, `openspec/specs/`. + +- [ ] 2.1 Archive the change + - Verification: `uv run devops openspec archive 2662-mask-url-user-name-without-password` + +## Workflow follow-up + +- Pending a person: review the diff in tuicr (tuicr -r origin/main..HEAD). diff --git a/src/devops_cli/security/sanitizer.py b/src/devops_cli/security/sanitizer.py index 67142e1b0..b8028d138 100644 --- a/src/devops_cli/security/sanitizer.py +++ b/src/devops_cli/security/sanitizer.py @@ -1,4 +1,24 @@ -"""Universal secret masking, credential redaction, and token sanitization engine.""" +"""Universal secret masking, credential redaction, and token sanitization engine. + +The userinfo of a URL is masked, whatever the scheme (#2662). A URL parser finds it, never a +pattern: `urllib.parse.urlsplit` in free text (`redact_text`) and `httpx2.URL` in a single URL +(`mask_uri_credentials`). + +- A user name and a password: `redact_text` shows `:`. + `mask_uri_credentials` keeps the user name and shows `user:***`. +- A password with no user name: `redact_text` shows `:`, and + `mask_uri_credentials` shows `***`. +- A user name with no password: the user name is masked, as `` in `redact_text` and as + `***` in `mask_uri_credentials`. A service can take a token as the user name, with an empty + password (httpx2 sends such a URL as Basic authentication). A token and a harmless name such as + `git` in `ssh://git@host/` look the same to a parser, so the rule masks both. pip masks a user + name with no password in the same way. +- A URL with no userinfo stays as written. An `@` after the authority, as in a query, is not + userinfo. + +A user name with a `/`, `?` or `#` that is not percent-encoded is not valid userinfo. The parser +reads it as part of the host or the path, so the rule cannot mask it. +""" from __future__ import annotations @@ -64,13 +84,20 @@ def _mask_url_candidate(match: re.Match[str]) -> str: except Exception: return "" if has_at else candidate + # The userinfo starts right after "://" and the parser gives its parts as written. + userinfo_start = candidate.find("://") + 3 if parts.password is not None: - scheme_end = candidate.find("://") + 3 userinfo_len = len(parts.username or "") + 1 + len(parts.password) return ( - f"{candidate[:scheme_end]}" + f"{candidate[:userinfo_start]}" f":" - f"{candidate[scheme_end + userinfo_len :]}" + f"{candidate[userinfo_start + userinfo_len :]}" + ) + if parts.username: + return ( + f"{candidate[:userinfo_start]}" + f"" + f"{candidate[userinfo_start + len(parts.username) :]}" ) if has_at: @@ -388,7 +415,12 @@ def mask_private_hosts(text: str) -> str: def redact_text(text: str, syntax: Literal["code", "data"] | str = "data") -> str: - """Sanitize and redact known credential and secret patterns in the input text.""" + """Sanitize and redact known credential and secret patterns in the input text. + + A URL in the text has its userinfo masked, whatever the scheme: `:` + for a user and a password, and `` for a user with no password. See the module + docstring for the rule. + """ if not text: return "" patterns = _SECRET_PATTERNS_CODE if _is_code_syntax(syntax) else _SECRET_PATTERNS_DATA @@ -442,7 +474,12 @@ def mask_secret_named_values(data: Any) -> Any: def mask_uri_credentials(uri: str) -> str: - """Mask password or credentials in URL/URI strings while preserving scheme and host.""" + """Mask the credentials in the userinfo of a URL/URI, and keep everything else. + + A user beside a password stays and the password becomes `***`. A password with no user, and + a user with no password, become `***` whole: a service can take a token as the user name, and + the parser cannot tell a token from a harmless name such as `git`. See the module docstring. + """ if not uri: return "" try: @@ -451,9 +488,9 @@ def mask_uri_credentials(uri: str) -> str: return "" if "@" in uri and not u.userinfo: return "" - if u.password: - if u.username: - return str(u.copy_with(username=u.username, password=_URL_MASK)) + if u.username and u.password: + return str(u.copy_with(username=u.username, password=_URL_MASK)) + if u.username or u.password: return str(u.copy_with(username=_URL_MASK, password=None)) return uri diff --git a/tests/ai/client/test_url_credential_masking.py b/tests/ai/client/test_url_credential_masking.py index fa517fdbd..c4ef940bd 100644 --- a/tests/ai/client/test_url_credential_masking.py +++ b/tests/ai/client/test_url_credential_masking.py @@ -203,22 +203,37 @@ def _eval_ssrf_fail_closed(input_str: str) -> Any: ( "custom://myuser@example.com/path", "mask_uri", - "custom://myuser@example.com/path", + "custom://***@example.com/path", ), ( "custom://myuser@example.com/path", "redact_text", - "custom://myuser@example.com/path", + "custom://@example.com/path", ), ( "https://ghp_x@example.com/o/r", "mask_uri", - "https://ghp_x@example.com/o/r", + "https://***@example.com/o/r", ), ( "https://ghp_x@example.com/o/r", "redact_text", - "https://ghp_x@example.com/o/r", + "https://@example.com/o/r", + ), + ( + "http://tok_0123456789abcdef@[2001:db8::1]:11434/api/embed", + "mask_uri", + "http://***@[2001:db8::1]:11434/api/embed", + ), + ( + "http://tok_0123456789abcdef@[2001:db8::1]:11434/api/embed", + "redact_text", + "http://@[2001:db8::1]:11434/api/embed", + ), + ( + '["http://example.com","https://tok123@example.com/x"]', + "redact_text", + '["http://example.com","https://@example.com/x"]', ), ( "http://[::1]:4318", diff --git a/tests/k8s/test_gitea_doctor.py b/tests/k8s/test_gitea_doctor.py index eec8c4874..bd75cb4a0 100644 --- a/tests/k8s/test_gitea_doctor.py +++ b/tests/k8s/test_gitea_doctor.py @@ -524,8 +524,8 @@ def test_a_wrong_answer_through_the_tunnel_fails_its_check( ), pytest.param( GiteaServer(ssh_host=HOST, ssh_form="ssh://git@{host}:2222/gitea_admin/{name}.git"), - "ssh_url ssh://git@{host}:2222/gitea_admin/{name}.git names the tunnel host, which " - "carries no SSH", + "ssh_url ssh://@{host}:2222/gitea_admin/{name}.git names the tunnel " + "host, which carries no SSH", id="ssh-url-form-names-the-tunnel-host", ), ], diff --git a/tests/security/test_redaction_properties.py b/tests/security/test_redaction_properties.py index 34d6c2cc6..03c3ce67e 100644 --- a/tests/security/test_redaction_properties.py +++ b/tests/security/test_redaction_properties.py @@ -50,3 +50,25 @@ def test_a_uri_keeps_everything_but_its_password( assert mask_uri_credentials(f"{scheme}://{user}:{password}@{host}{rest}") == ( f"{scheme}://{user}:***@{host}{rest}" ) + + +@given( + scheme=st.sampled_from(["https", "http", "postgresql", "redis", "ssh"]), + user=st.text(alphabet="abcdefghijklmnopqrstuvwxyz0123456789", min_size=1, max_size=12), + host=st.sampled_from( + ["example.com", "db.example.com:5432", "192.0.2.10", "[2001:db8::1]:8080"] + ), + rest=st.sampled_from(["", "/", "/x", "/db?sslmode=require", "/x#top"]), + syntax=syntaxes, +) +def test_a_url_user_with_no_password_is_masked_and_the_rest_stays( + scheme: str, user: str, host: str, rest: str, syntax: str +) -> None: + """The user name never survives, as it can be a token. The scheme, host, path, query and + fragment stay.""" + url = f"{scheme}://{user}@{host}{rest}" + + masked_text = mask_secrets(f"see {url} now", syntax=syntax) + + assert masked_text == f"see {scheme}://@{host}{rest} now" + assert mask_uri_credentials(url) == f"{scheme}://***@{host}{rest}" diff --git a/tests/security/test_secret_redaction.py b/tests/security/test_secret_redaction.py index ca65a5472..77ce327d0 100644 --- a/tests/security/test_secret_redaction.py +++ b/tests/security/test_secret_redaction.py @@ -111,15 +111,72 @@ def test_mask_secrets_expanded_patterns() -> None: "custom://***@myhost/path", id="custom-scheme-password-without-user", ), - pytest.param("custom://myuser@myhost/path", "custom://myuser@myhost/path", id="user-only"), + pytest.param("custom://myuser@myhost/path", "custom://***@myhost/path", id="user-only"), + pytest.param( + "https://tok_0123456789abcdef@example.com:11434/api/embed", + "https://***@example.com:11434/api/embed", + id="token-as-user", + ), + pytest.param( + "http://example.com:11434/api/embed", + "http://example.com:11434/api/embed", + id="no-userinfo", + ), + pytest.param( + "http://:@example.com/", "http://:@example.com/", id="empty-user-and-password" + ), ], ) -def test_a_uri_password_is_masked_and_a_user_alone_is_kept(uri: str, masked: str) -> None: - """Only a password is masked; with no user, the whole userinfo becomes `***` and no empty - user is left before it.""" +def test_a_uri_password_and_a_user_alone_are_masked(uri: str, masked: str) -> None: + """A user beside a password stays and the password becomes `***`. A user alone is masked + whole, as a user with no password can be a token: the userinfo becomes `***`, as it does + for a password with no user, and no empty user is left before it.""" assert mask_uri_credentials(uri) == masked +@pytest.mark.parametrize( + ("text", "masked"), + [ + pytest.param( + "error calling http://tok_0123456789abcdef@example.com:11434/api/embed failed", + "error calling http://@example.com:11434/api/embed failed", + id="token-as-user", + ), + pytest.param( + "ssh://git@example.com/org/repo.git", + "ssh://@example.com/org/repo.git", + id="harmless-user-is-masked-too", + ), + pytest.param( + '{"url": "HTTPS://tok123@example.com/x?a=b#c"}', + '{"url": "HTTPS://@example.com/x?a=b#c"}', + id="inside-json-with-query-and-fragment", + ), + pytest.param( + "https://example.com/login?next=https://tok123@example.com/x", + "https://example.com/login?next=https://@example.com/x", + id="url-inside-a-query", + ), + pytest.param( + "http://user:pw12345@example.com/x", + "http://:@example.com/x", + id="user-and-password", + ), + pytest.param( + "http://example.com:11434/p?email=a@example.com", + "http://example.com:11434/p?email=a@example.com", + id="at-sign-after-the-authority", + ), + pytest.param("http://@example.com/x", "http://@example.com/x", id="empty-user"), + ], +) +@pytest.mark.parametrize("syntax", ["data", "code"]) +def test_redact_text_masks_a_url_user_with_no_password(text: str, masked: str, syntax: str) -> None: + """The user name of a URL is masked when the URL has no password, in data and code syntax. + The scheme, the host, the port, the path, the query and the fragment stay.""" + assert redact_text(text, syntax=syntax) == masked + + def test_devops_cli_error_masks_message_and_details() -> None: """Verify that DevOpsCLIError and all subclasses automatically redact secrets.""" from devops_cli.exceptions.base import DevOpsCLIError From 4bca30df9e904e030b87ba12bd089addbc11d420 Mon Sep 17 00:00:00 2001 From: Daniel Petty Date: Thu, 5 Nov 2026 17:47:07 +0000 Subject: [PATCH 2/3] docs(security): state the fail-closed result of mask_uri_credentials The spec, the design and the module docstring said that both masking functions return a URL with no user name as it was. mask_uri_credentials returns for a text that has an @ and no userinfo (#1109). The texts now name each function, and mask_uri_credentials has a test for both forms. Plans #2662. --- .../design.md | 2 +- .../specs/secret-masking/spec.md | 16 ++++++++++------ src/devops_cli/security/sanitizer.py | 5 +++-- tests/ai/client/test_url_credential_masking.py | 15 +++++++++++++++ 4 files changed, 29 insertions(+), 9 deletions(-) diff --git a/openspec/changes/2662-mask-url-user-name-without-password/design.md b/openspec/changes/2662-mask-url-user-name-without-password/design.md index c7d8cca32..cdf4dadcb 100644 --- a/openspec/changes/2662-mask-url-user-name-without-password/design.md +++ b/openspec/changes/2662-mask-url-user-name-without-password/design.md @@ -43,7 +43,7 @@ The user name is masked in `redact_text` and in `mask_uri_credentials`, in every ### 4. An empty user name stays -`http://@example.com/x` has an empty user name and no password. It holds no credential, so both functions return it as it was. The form `http://:@example.com/` also stays in `mask_uri_credentials`. +`http://@example.com/x` has an empty user name and no password. It holds no credential, so `redact_text` returns it as it was. `mask_uri_credentials` keeps its fail-closed rule from #1109 and returns `` for a text that has an `@` and no userinfo, and this change does not touch that rule. The form `http://:@example.com/` has userinfo, so `mask_uri_credentials` returns it as it was. ## Risks / Trade-offs diff --git a/openspec/changes/2662-mask-url-user-name-without-password/specs/secret-masking/spec.md b/openspec/changes/2662-mask-url-user-name-without-password/specs/secret-masking/spec.md index 7220265b2..ffd9cf68d 100644 --- a/openspec/changes/2662-mask-url-user-name-without-password/specs/secret-masking/spec.md +++ b/openspec/changes/2662-mask-url-user-name-without-password/specs/secret-masking/spec.md @@ -19,17 +19,21 @@ The secret masking system SHALL mask the user name of a URL that has a user name - **WHEN** `mask_uri_credentials` processes `https://tok_0123456789abcdef@example.com:11434/api/embed` - **THEN** the result is `https://***@example.com:11434/api/embed` -### Requirement: A URL with no user name stays as written -The secret masking system SHALL leave a URL as written when it has no userinfo, when its user name is empty and it has no password, or when its only `@` is after the authority. +### Requirement: A URL with no user name stays as written in a text +`redact_text` and `mask_secrets` SHALL leave a URL as written when it has no userinfo, when its user name is empty and it has no password, or when its only `@` is after the authority. `mask_uri_credentials` SHALL fail closed for a text that has an `@` but no userinfo, and SHALL return ``. -#### Scenario: An at sign in a query -- **WHEN** secret masking processes `http://example.com:11434/p?email=a@example.com` +#### Scenario: An at sign in a query in a text +- **WHEN** `redact_text` processes `http://example.com:11434/p?email=a@example.com` - **THEN** the text comes back unchanged -#### Scenario: An empty user name -- **WHEN** secret masking processes `http://@example.com/x` +#### Scenario: An empty user name in a text +- **WHEN** `redact_text` processes `http://@example.com/x` - **THEN** the text comes back unchanged +#### Scenario: An at sign and no userinfo in one URI +- **WHEN** `mask_uri_credentials` processes `http://@example.com/x` or `http://example.com:11434/p?email=a@example.com` +- **THEN** the result is `` + ### Requirement: A URL with a user name and a password keeps its masking The secret masking system SHALL mask both the user name and the password of a URL that has both. `redact_text` SHALL show `:`. `mask_uri_credentials` SHALL keep the user name and show `user:***`. A password with no user name SHALL become `***` in `mask_uri_credentials`. diff --git a/src/devops_cli/security/sanitizer.py b/src/devops_cli/security/sanitizer.py index b8028d138..8f1441abb 100644 --- a/src/devops_cli/security/sanitizer.py +++ b/src/devops_cli/security/sanitizer.py @@ -13,8 +13,9 @@ password (httpx2 sends such a URL as Basic authentication). A token and a harmless name such as `git` in `ssh://git@host/` look the same to a parser, so the rule masks both. pip masks a user name with no password in the same way. -- A URL with no userinfo stays as written. An `@` after the authority, as in a query, is not - userinfo. +- A URL with no userinfo stays as written in `redact_text`. An `@` after the authority, as in a + query, is not userinfo. `mask_uri_credentials` fails closed for a text that has an `@` and no + userinfo, and returns `` (#1109). A user name with a `/`, `?` or `#` that is not percent-encoded is not valid userinfo. The parser reads it as part of the host or the path, so the rule cannot mask it. diff --git a/tests/ai/client/test_url_credential_masking.py b/tests/ai/client/test_url_credential_masking.py index c4ef940bd..e45601ef2 100644 --- a/tests/ai/client/test_url_credential_masking.py +++ b/tests/ai/client/test_url_credential_masking.py @@ -110,6 +110,21 @@ def _eval_ssrf_fail_closed(input_str: str) -> Any: "mask_uri", "http://***@example.com:8080/data", ), + ( + "http://@example.com/x", + "mask_uri", + "", + ), + ( + "http://example.com:8080/p?email=a@example.com", + "mask_uri", + "", + ), + ( + "http://@example.com/x", + "redact_text", + "http://@example.com/x", + ), ( "https://user:password@ss@example.com/x", "redact_text", From 68fbde0b24cfbd518b62c5995d4f3d860f429677 Mon Sep 17 00:00:00 2001 From: Daniel Petty Date: Thu, 5 Nov 2026 18:05:24 +0000 Subject: [PATCH 3/3] docs(openspec): archive 2662-mask-url-user-name-without-password Closes #2662 --- .../.openspec.yaml | 0 .../design.md | 0 .../proposal.md | 0 .../specs/secret-masking/spec.md | 0 .../tasks.md | 2 +- openspec/specs/secret-masking/spec.md | 49 +++++++++++++++++++ 6 files changed, 50 insertions(+), 1 deletion(-) rename openspec/changes/{2662-mask-url-user-name-without-password => archive/2026-11-05-2662-mask-url-user-name-without-password}/.openspec.yaml (100%) rename openspec/changes/{2662-mask-url-user-name-without-password => archive/2026-11-05-2662-mask-url-user-name-without-password}/design.md (100%) rename openspec/changes/{2662-mask-url-user-name-without-password => archive/2026-11-05-2662-mask-url-user-name-without-password}/proposal.md (100%) rename openspec/changes/{2662-mask-url-user-name-without-password => archive/2026-11-05-2662-mask-url-user-name-without-password}/specs/secret-masking/spec.md (100%) rename openspec/changes/{2662-mask-url-user-name-without-password => archive/2026-11-05-2662-mask-url-user-name-without-password}/tasks.md (98%) diff --git a/openspec/changes/2662-mask-url-user-name-without-password/.openspec.yaml b/openspec/changes/archive/2026-11-05-2662-mask-url-user-name-without-password/.openspec.yaml similarity index 100% rename from openspec/changes/2662-mask-url-user-name-without-password/.openspec.yaml rename to openspec/changes/archive/2026-11-05-2662-mask-url-user-name-without-password/.openspec.yaml diff --git a/openspec/changes/2662-mask-url-user-name-without-password/design.md b/openspec/changes/archive/2026-11-05-2662-mask-url-user-name-without-password/design.md similarity index 100% rename from openspec/changes/2662-mask-url-user-name-without-password/design.md rename to openspec/changes/archive/2026-11-05-2662-mask-url-user-name-without-password/design.md diff --git a/openspec/changes/2662-mask-url-user-name-without-password/proposal.md b/openspec/changes/archive/2026-11-05-2662-mask-url-user-name-without-password/proposal.md similarity index 100% rename from openspec/changes/2662-mask-url-user-name-without-password/proposal.md rename to openspec/changes/archive/2026-11-05-2662-mask-url-user-name-without-password/proposal.md diff --git a/openspec/changes/2662-mask-url-user-name-without-password/specs/secret-masking/spec.md b/openspec/changes/archive/2026-11-05-2662-mask-url-user-name-without-password/specs/secret-masking/spec.md similarity index 100% rename from openspec/changes/2662-mask-url-user-name-without-password/specs/secret-masking/spec.md rename to openspec/changes/archive/2026-11-05-2662-mask-url-user-name-without-password/specs/secret-masking/spec.md diff --git a/openspec/changes/2662-mask-url-user-name-without-password/tasks.md b/openspec/changes/archive/2026-11-05-2662-mask-url-user-name-without-password/tasks.md similarity index 98% rename from openspec/changes/2662-mask-url-user-name-without-password/tasks.md rename to openspec/changes/archive/2026-11-05-2662-mask-url-user-name-without-password/tasks.md index 2545b90e0..8e9e6d75e 100644 --- a/openspec/changes/2662-mask-url-user-name-without-password/tasks.md +++ b/openspec/changes/archive/2026-11-05-2662-mask-url-user-name-without-password/tasks.md @@ -15,7 +15,7 @@ Kind: behavior. Reads: `src/devops_cli/security/sanitizer.py`, `tests/security/t Kind: documentation. Reads: none. Changes: `openspec/changes/`, `openspec/specs/`. -- [ ] 2.1 Archive the change +- [x] 2.1 Archive the change - Verification: `uv run devops openspec archive 2662-mask-url-user-name-without-password` ## Workflow follow-up diff --git a/openspec/specs/secret-masking/spec.md b/openspec/specs/secret-masking/spec.md index 01b8ffc4a..2c5fea986 100644 --- a/openspec/specs/secret-masking/spec.md +++ b/openspec/specs/secret-masking/spec.md @@ -105,3 +105,52 @@ The secret masking system SHALL mask the value of an assignment even when the va #### Scenario: An assignment operator - **WHEN** secret masking processes `password:=correcthorse`, `token:=abcdefg` or `token: =correcthorse` - **THEN** the result is `password=` or `token=` + +### Requirement: The user name of a URL is masked when the URL has no password +The secret masking system SHALL mask the user name of a URL that has a user name and no password, whatever the scheme. `redact_text` and `mask_secrets` SHALL show `` in place of the user name, in data and code syntax. `mask_uri_credentials` SHALL show `***`. The scheme, the host, the port, the path, the query and the fragment SHALL stay as written. + +#### Scenario: A token used as the user name in a text +- **WHEN** secret masking processes `error calling http://tok_0123456789abcdef@example.com:11434/api/embed failed` +- **THEN** the result is `error calling http://@example.com:11434/api/embed failed` + +#### Scenario: A harmless user name +- **WHEN** secret masking processes `ssh://git@example.com/org/repo.git` +- **THEN** the result is `ssh://@example.com/org/repo.git` + +#### Scenario: A user name in a URL with an IPv6 host, a query and a fragment +- **WHEN** secret masking processes `http://tok123@[2001:db8::1]:11434/x?a=b#c` +- **THEN** the result is `http://@[2001:db8::1]:11434/x?a=b#c` + +#### Scenario: One URI with a user name and no password +- **WHEN** `mask_uri_credentials` processes `https://tok_0123456789abcdef@example.com:11434/api/embed` +- **THEN** the result is `https://***@example.com:11434/api/embed` + +### Requirement: A URL with no user name stays as written in a text +`redact_text` and `mask_secrets` SHALL leave a URL as written when it has no userinfo, when its user name is empty and it has no password, or when its only `@` is after the authority. `mask_uri_credentials` SHALL fail closed for a text that has an `@` but no userinfo, and SHALL return ``. + +#### Scenario: An at sign in a query in a text +- **WHEN** `redact_text` processes `http://example.com:11434/p?email=a@example.com` +- **THEN** the text comes back unchanged + +#### Scenario: An empty user name in a text +- **WHEN** `redact_text` processes `http://@example.com/x` +- **THEN** the text comes back unchanged + +#### Scenario: An at sign and no userinfo in one URI +- **WHEN** `mask_uri_credentials` processes `http://@example.com/x` or `http://example.com:11434/p?email=a@example.com` +- **THEN** the result is `` + +### Requirement: A URL with a user name and a password keeps its masking +The secret masking system SHALL mask both the user name and the password of a URL that has both. `redact_text` SHALL show `:`. `mask_uri_credentials` SHALL keep the user name and show `user:***`. A password with no user name SHALL become `***` in `mask_uri_credentials`. + +#### Scenario: A user name and a password in a text +- **WHEN** secret masking processes `http://user:pw12345@example.com/x` +- **THEN** the result is `http://:@example.com/x` + +#### Scenario: One URI with a user name and a password +- **WHEN** `mask_uri_credentials` processes `https://user:secret123@example.com:8443/api` +- **THEN** the result is `https://user:***@example.com:8443/api` + +#### Scenario: One URI with a password and no user name +- **WHEN** `mask_uri_credentials` processes `http://:supersecretpass@example.com:8080/data` +- **THEN** the result is `http://***@example.com:8080/data`