From c81e074fe3db4830f294abed76db05d13c2a1f90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=A8=E5=A0=83?= Date: Wed, 9 Sep 2026 17:45:57 +0800 Subject: [PATCH 1/3] fix(agent_hub): BUG-0903 split agent CLI unrecognized-args hint by arg kind --- ms_agent/cli/cli.py | 12 ++++++++---- tests/cli/test_cli_entry.py | 6 +++++- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/ms_agent/cli/cli.py b/ms_agent/cli/cli.py index cc8ecbf18..5182303e8 100644 --- a/ms_agent/cli/cli.py +++ b/ms_agent/cli/cli.py @@ -49,10 +49,14 @@ def run_cmd(): # leftover argument is always a user mistake (e.g. a bare path that would # silently fall back to the framework default workspace on upload). if unknown and isinstance(cmd, AgentCMD): - parser.error( - f'unrecognized arguments: {" ".join(unknown)} ' - '(`ms-agent agent` takes no positional arguments; ' - 'use --local-dir to specify a local directory)') + # Only a leftover positional argument warrants the --local-dir hint; + # for a misspelled option keep the plain unrecognized-arguments error. + if any(not u.startswith('-') for u in unknown): + parser.error( + f'unrecognized arguments: {" ".join(unknown)} ' + '(`ms-agent agent` takes no positional arguments; ' + 'use --local-dir to specify a local directory)') + parser.error(f'unrecognized arguments: {" ".join(unknown)}') cmd.execute() diff --git a/tests/cli/test_cli_entry.py b/tests/cli/test_cli_entry.py index 3ddc86c81..3acb6a41b 100644 --- a/tests/cli/test_cli_entry.py +++ b/tests/cli/test_cli_entry.py @@ -47,7 +47,11 @@ def _fail_execute(self): with pytest.raises(SystemExit) as exc_info: run_cmd() assert exc_info.value.code == 2 - assert 'unrecognized arguments: --dry-runn' in capsys.readouterr().err + stderr = capsys.readouterr().err + assert 'unrecognized arguments: --dry-runn' in stderr + # A misspelled option must not get the positional-argument hint. + assert '--local-dir' not in stderr + assert 'no positional arguments' not in stderr def test_valid_agent_command_still_dispatches(monkeypatch): From a18510c08c7752bfa6cd0c2a8637f76f24018f98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=A8=E5=A0=83?= Date: Thu, 10 Sep 2026 15:46:55 +0800 Subject: [PATCH 2/3] fix(agent_hub): BUG redact outbound secrets by content, not file path --- ms_agent/agent_hub/_commands.py | 16 +- ms_agent/agent_hub/_secrets.py | 751 +++++++++++++++++++++ ms_agent/agent_hub/_sync.py | 38 +- ms_agent/agent_hub/_watcher.py | 9 +- ms_agent/agent_hub/_workspace.py | 163 +++++ ms_agent/agent_hub/frameworks/openhuman.py | 152 +---- tests/agent_hub/test_cli.py | 109 +++ tests/agent_hub/test_secrets.py | 466 +++++++++++++ tests/agent_hub/test_workspace.py | 182 +++++ 9 files changed, 1734 insertions(+), 152 deletions(-) create mode 100644 ms_agent/agent_hub/_secrets.py create mode 100644 tests/agent_hub/test_secrets.py diff --git a/ms_agent/agent_hub/_commands.py b/ms_agent/agent_hub/_commands.py index 1ab9e3761..30c093953 100644 --- a/ms_agent/agent_hub/_commands.py +++ b/ms_agent/agent_hub/_commands.py @@ -426,9 +426,11 @@ def cmd_upload( # boilerplate is never pushed -- keeps upload and convert 1:1-consistent # about what "the user's own files" are. from ._sync import drop_unchanged_defaults, sanitize_outbound + redacted: list = [] try: resources = drop_unchanged_defaults( - sanitize_outbound(resources, spec), framework, spec) + sanitize_outbound(resources, spec, findings=redacted), framework, + spec) except ValueError as e: # Fail-closed sanitize: a config file that cannot be parsed cannot be # verified secret-free -- abort instead of pushing plaintext keys. @@ -450,6 +452,18 @@ def cmd_upload( headers=['FILE', 'SIZE'], color=display.COLOR_WRITTEN, ) + if redacted: + # Reported before the dry-run return so `--dry-run` shows exactly what + # a real upload would strip. A Finding never carries the secret text. + display.table( + 'Secrets redacted', + [(f.rel, f.kind, f'line {f.line}' if f.line else 'structural') + for f in sorted(redacted)], + headers=['FILE', 'KIND', 'WHERE'], + color=display.COLOR_MERGED, + note=f'{len(redacted)} secret value(s) replaced with ' + f'[REDACTED:*] in the uploaded copy; local files are untouched.', + ) if dry_run: print('\n[dry-run] nothing uploaded.') diff --git a/ms_agent/agent_hub/_secrets.py b/ms_agent/agent_hub/_secrets.py new file mode 100644 index 000000000..fe7f7c266 --- /dev/null +++ b/ms_agent/agent_hub/_secrets.py @@ -0,0 +1,751 @@ +# Copyright (c) ModelScope Contributors. All rights reserved. +"""Content-driven outbound secret redaction (BUG-0909-01). + +Why this module exists +---------------------- +:meth:`WorkspaceSpec.sanitize_outbound_file` decides *whether* to clean a file +by its PATH: each framework whitelists its own root config (ms-agent +``settings.json`` / ``mcp.json``, qwenpaw ``agent.json``, hermes +``config.yaml``, openhuman ``config.toml``) and returns every other collected +file verbatim -- openclaw, nanobot and qoder define no hook at all. The +collect patterns, however, take ``skills/*`` recursively plus the persona and +memory documents, so a key that an AI assistant wrote into a skill script, a +skill-local ``mcp.json``, ``SOUL.md`` or ``MEMORY.md`` was uploaded verbatim +into the remote repo and its git history. + +This layer decides on CONTENT instead of path. It runs *after* the +per-framework hook (see :func:`ms_agent.agent_hub._sync.sanitize_outbound`), +so the structural cleaning and the fail-closed refusals for the known config +files keep their behavior, and every framework -- present or future -- is +covered without editing a single spec. + +Two tiers +--------- +* **Tier A (config-shaped)** -- a file that IS configuration, by name + (``mcp.json``, ``config.yaml``, ...) or by shape (its parsed JSON/YAML/TOML + carries an ``mcpServers`` / ``mcp_servers`` mapping), is cleaned with the + structural scrubbers in :mod:`._workspace`. A shape-triggered file is + cleaned only inside that MCP subtree: the shared vocabulary blanks any key + named ``tokens`` / ``keys`` / ``session_id``, which would destroy legitimate + data in a skill's JSON fixture or a memory dump. +* **Tier B (everything else)** -- documents, scripts, notebooks, JSONL: only + high-confidence secret *values* are replaced with a ``[REDACTED:]`` + marker. The bag rules (``env`` / ``headers`` cleared wholesale) never apply + here, and the name vocabulary is narrower than :func:`is_secret_key` + (``max_tokens`` / ``page_token`` / ``session_id`` are pagination and + telemetry fields, not credentials). + +Contract +-------- +* **Never raises.** The watch daemon swallows exceptions and keeps polling + (``_watcher._poll_once``), so a raise here would silently stop all syncing. +* **Returns the original bytes object when nothing was redacted.** + Re-serializing a parsed config reformats it, which breaks both + :func:`._sync.drop_unchanged_defaults` (byte comparison against the + framework default templates) and the sha256 idempotent-skip in + ``push_mirror`` / ``push_resources``. +* **Deterministic and idempotent.** The watcher stores the sha256 of the + SANITIZED bytes as its sync baseline, so an unstable rewrite would re-push + every cycle; a ``[REDACTED:...]`` marker never re-matches any rule. +* **Outbound only.** ``sanitize_inbound_file`` also serves local ``convert`` + writes, where redaction would strip the user's own keys from the converted + agent and leave it unable to run. +""" +from __future__ import annotations + +import base64 +import binascii +import hashlib +import json +import math +import re +from collections import Counter, OrderedDict +from typing import NamedTuple + +from ms_agent.utils.logger import get_logger +from ._workspace import (scrub_json_secrets, scrub_toml_secrets, + scrub_yaml_secrets) + +logger = get_logger() + +__all__ = ['Finding', 'redact_outbound', 'redact_text'] + + +class Finding(NamedTuple): + """One redacted secret. + + Carries the *kind* and the key name, never the secret text itself, so the + upload report and the watch log cannot leak what they are reporting. + ``line`` is 1-based; Tier A (structural config cleaning) reports line 0 + because a structural scrub has no single location. + """ + rel: str + kind: str + line: int + name: str + + +# --------------------------------------------------------------------------- +# Tier B: high-confidence secret VALUES +# --------------------------------------------------------------------------- + +# Vendor-prefixed credentials. The prefix is decisive on its own, so only the +# placeholder / variable-reference gate applies to these matches -- a real key +# may legitimately have no digit or an unusual charset. The lookbehind keeps +# ``sk-`` from firing inside ordinary words (``task-tracking``, +# ````). +_VENDOR_TOKEN_RE = re.compile( + r'(?[A-Za-z0-9._~+/=\-]{8,})') + +# ``NAME: VALUE`` / ``NAME=VALUE`` with optional quoting on either side, so it +# covers JSON, YAML, shell exports, Python assignments and markdown lists. +_ASSIGN_RE = re.compile(r'(?P["\']?)' + r'(?P[A-Za-z0-9_][A-Za-z0-9_.\-]{1,62})' + r'(?P["\']?)' + r'(?P[ \t]*[:=][ \t]*)' + r'(?P["\']?)' + r'(?P[^\s"\'`,;)\]}>]+)') + +# ``--flag VALUE`` / ``--flag=VALUE`` / ``-f VALUE`` on a documented command +# line. +_FLAG_RE = re.compile( + r'(?[A-Za-z][A-Za-z0-9_.\-]{1,62})' + r'(?:[= \t]+(?P["\']?)' + r'(?P[^\s"\'`,;)\]}>]+))?') + +# Absolute URLs in free text. The charset stops at markdown/link punctuation +# so `[doc](https://h/p)` and trailing ``.,;:`` are not swallowed. +_FREE_URL_RE = re.compile(r'(?[A-Za-z][A-Za-z0-9+.\-]*://)' + r'(?P[^\s`"\'<>()\[\]{},;|\\^]+)') + +# A base64 payload that decodes to a secret ("decode this at runtime" persona +# instructions). Bounded length keeps data URIs and minified bundles out. +_BASE64_RE = re.compile(r'(?[A-Za-z0-9+/]{20,512}={0,2})' + r'(?![A-Za-z0-9+/=])') +_BASE64_MAX_ATTEMPTS = 400 + +# Values that are documentation placeholders rather than credentials. Only +# unambiguous markers qualify: a 5+ character word, a repeated-character run +# (``xxxx``), or filler digits. Short words (``foo``, ``test``, ``none``) are +# deliberately absent -- they occur inside a random base62 credential often +# enough (~1 in 1000) that treating them as placeholders would let real keys +# through, and a missed key is worse than a redacted doc example. +_PLACEHOLDER_RE = re.compile( + r'(?:your|yours|xxx+|yyy+|zzz+|example|dummy|fake|sample|placeholder|' + r'changeme|change_me|redacted|todo|fixme|localhost|something|whatever|' + r'unknown|mysecret|secretvalue|000000|123456)', re.IGNORECASE) + +# Characters that mark a value as an expression / path / reference rather than +# a credential (``keyvaultref:$SECRET_URI``, ``options?.pageToken``, +# ``${API_KEY}``, ``/etc/keys/a.pem``). +_BAD_VALUE_CHARS = frozenset('$={}?*()@/\\`|&<>:,#%') + +# A fully base64-ish value is allowed to carry ``/``, ``+`` and ``=`` padding. +_BASE64ISH_RE = re.compile(r'^[A-Za-z0-9+/]{16,}={0,2}$') + +_CAMEL_RE = re.compile(r'[a-z][A-Z]') + +# Tier B name vocabulary, in three strengths. Deliberately NARROWER than +# :func:`._workspace.is_secret_key`: bare plural ``tokens`` / ``keys`` and the +# pagination / telemetry spellings in :data:`_NAME_DENY` are ordinary data +# fields (a nanobot ``memory/history.jsonl`` cursor, an LLM ``max_tokens`` +# setting) whose values are high-entropy digit-bearing strings -- exactly what +# a credential looks like. ``sk`` is dropped too: it is meaningless as a key +# name and real ``sk-`` values are caught by :data:`_VENDOR_TOKEN_RE`. +# +# STRONG names (``api_key``, ``access_token``, ``client_secret``, ...) are +# unambiguous: a credential is what they hold. +_STRONG_NAME_RE = re.compile( + r'(?:^|[_\-.])(?:' + r'api[_-]?keys?|apikeys?|access[_-]?keys?|secret[_-]?keys?|' + r'client[_-]?secrets?|access[_-]?tokens?|auth[_-]?tokens?|' + r'id[_-]?tokens?|refresh[_-]?tokens?|token[_-]?file|' + r'authorization|passphrases?|private[_-]?keys?|' + r'auth[_-]?key|secret[_-]?key|master[_-]?key|signing[_-]?key)$', + re.IGNORECASE) + +# WEAK names are BARE singulars. They do hold credentials in a memory note +# ("gateway token: ...") or a documented command line (``--token ...``), but +# they are also ordinary data-mapping fields (``{"key": "user_profile_2"}``), +# so they only fire against a stricter value gate. The same word WITH a +# qualifier (``github_token``, ``db_password``, ``smtp_passwd``) is +# unambiguous and counts as strong. +_WEAK_NAME_RE = re.compile( + r'(?:^|[_\-.])(?:keys?|tokens?|secrets?|passwords?|passwd|' + r'credentials?|cookies?|bearer)$', re.IGNORECASE) + +# Bare stems, singularized: a name whose normalized form IS one of these (and +# nothing more) is weak. +_WEAK_BARE_STEMS = frozenset(( + 'key', + 'token', + 'secret', + 'password', + 'passwd', + 'credential', + 'cookie', + 'bearer', +)) + +# Names that are never a trigger. Plurals are data fields (an LLM +# ``max_tokens`` setting, a nanobot ``memory/history.jsonl`` cursor) whose +# values are high-entropy digit-bearing strings -- exactly what a credential +# looks like -- and the metadata spellings describe a secret without being +# one. ``sk`` is absent from every vocabulary for the same reason: it is +# meaningless as a key name, and real ``sk-`` values are caught by +# :data:`_VENDOR_TOKEN_RE`. +_NAME_DENY = frozenset(( + # pagination / telemetry cursors + 'page_token', + 'next_token', + 'next_page_token', + 'continuation_token', + 'cursor', + 'page_cursor', + 'session_id', + 'request_id', + 'trace_id', + 'span_id', + 'correlation_id', + # plural data fields + 'tokens', + 'keys', + 'secrets', + 'credentials', + 'cookies', + # credential metadata + 'key_id', + 'key_name', + 'key_type', + 'key_path', + 'key_alias', + 'token_type', + 'token_name', + 'secret_name', + 'secret_id', +)) + +# Name strength: 0 = never a trigger, 1 = weak (strict value gate), +# 2 = strong. +_DENY, WEAK, STRONG = 0, 1, 2 + + +def name_strength(name: str) -> int: + """How much a key / flag *name* is allowed to drive Tier B redaction. + + ``name`` may be dotted or dashed (``model.api_key``, ``--auth-token``). + An explicit strong spelling wins first (``client_secrets`` and + ``access_tokens`` are credentials even though their last segment is a + denied plural), then the deny list, then the generic vocabulary -- where a + BARE singular is weak and a qualified one (``github_token``, + ``db_password``) is strong. Testing the last segment is what keeps + ``options.pageToken`` from being a trigger at all: there is no separator + before ``token``. + """ + full = name.strip().strip('\'"').lstrip('-').lower() + if not full: + return _DENY + normalized = full.replace('-', '_').replace('.', '_') + if _STRONG_NAME_RE.search(full): + return STRONG + if normalized in _NAME_DENY or normalized.split('_')[-1] in _NAME_DENY: + return _DENY + if _WEAK_NAME_RE.search(full): + if normalized.rstrip('s') in _WEAK_BARE_STEMS: + return WEAK + return STRONG + return _DENY + + +def _shannon_entropy(value: str) -> float: + """Bits per character of *value* (0.0 for the empty string).""" + if not value: + return 0.0 + total = len(value) + return -sum( + (c / total) * math.log2(c / total) for c in Counter(value).values()) + + +# Lowercase snake_case / kebab-case is how a data-mapping value or a doc +# placeholder spells itself (``user_profile_2``, ``my-super-secret-1``); real +# credentials are hex, base64 or mixed-case. Only applied to WEAK names, so a +# strongly named ``api_key`` keeps its value whatever the casing. +_SNAKE_CASE_RE = re.compile(r'^[a-z0-9]+(?:[_\-][a-z0-9]+)+$') + +# Letter-words with at most TRAILING digits: the shape of a variable or field +# name (``SendGridApiKey``, ``MySecretValue123``, ``options.pageToken``), not +# of a credential. A real key interleaves digits with letters +# (``S42bMemTokenLeak01``), which fails this shape and stays eligible. +_IDENTIFIER_SHAPE_RE = re.compile(r'^[A-Za-z][A-Za-z_]*[0-9]*$') + + +def _looks_real(value: str, quoted: bool, strength: int = STRONG) -> bool: + """Whether *value* looks like a real credential rather than a placeholder. + + This gate is what keeps the redactor from eating documentation. A value + must be long enough, carry at least one digit, be high-entropy, and must + not be a variable reference, a placeholder word, an expression, a path, or + a camelCase identifier (``SendGridApiKey``, ``options.pageToken``). Fully + base64-shaped values are exempt from the punctuation ban and the identifier + test, since real keys legitimately carry ``/``, ``+`` and ``=`` padding. + + A WEAK name (bare ``key`` / ``token`` / ``secret`` / ...) raises every + threshold and additionally refuses lowercase snake_case values. + """ + val = value.strip().strip('\'"`') + if not val: + return False + if val.startswith(('[', '<', '{', '$', '%', '#', '-', '+')): + return False + if '[REDACTED' in val: + return False + base64ish = bool(_BASE64ISH_RE.match(val)) + if not base64ish: + stripped = val.rstrip('=') + if any(c in _BAD_VALUE_CHARS for c in stripped): + return False + if _CAMEL_RE.search(val) and _IDENTIFIER_SHAPE_RE.match(val): + return False + if strength == WEAK and _SNAKE_CASE_RE.match(val): + return False + if _PLACEHOLDER_RE.search(val): + return False + if strength == WEAK: + min_len, min_entropy = (14, 3.3) if quoted else (16, 3.3) + else: + min_len, min_entropy = (12, 3.0) if quoted else (16, 3.2) + if len(val) < min_len: + return False + if len(set(val)) <= 3: + return False + if not any(c.isdigit() for c in val): + return False + return _shannon_entropy(val) >= min_entropy + + +def _marker(kind: str) -> str: + return f'[REDACTED:{kind}]' + + +def _line_of(text: str, pos: int) -> int: + return text.count('\n', 0, pos) + 1 + + +def _redact_vendor(text: str, hits: list) -> str: + + def repl(m): + value = m.group(0) + if _PLACEHOLDER_RE.search(value): + return value + hits.append(('api_key', _line_of(text, m.start()), '')) + return _marker('api_key') + + return _VENDOR_TOKEN_RE.sub(repl, text) + + +def _redact_jwt(text: str, hits: list) -> str: + + def repl(m): + hits.append(('jwt', _line_of(text, m.start()), '')) + return _marker('jwt') + + return _JWT_RE.sub(repl, text) + + +def _redact_bearer(text: str, hits: list) -> str: + + def repl(m): + token = m.group('token') + if not _looks_real(token, quoted=False): + return m.group(0) + hits.append(('bearer', _line_of(text, m.start('token')), '')) + return f'Bearer {_marker("bearer")}' + + return _BEARER_RE.sub(repl, text) + + +def _redact_urls(text: str, hits: list) -> str: + """Strip credentials from URLs embedded in free text. + + Lenient sibling of :func:`._workspace.scrub_url_secrets`, which is written + for config scalars and blanks a secret-named query parameter even when its + value is a variable reference (``?access_token=${token}`` in a converted + ``AGENTS.md``). Here every credential candidate passes :func:`_looks_real` + first, so documentation examples such as + ``postgresql+asyncpg://user:pass@host`` survive. + """ + + def repl(m): + rest = m.group('rest') + trail = '' + while rest and rest[-1] in '.,;:!?\'"': + trail = rest[-1] + trail + rest = rest[:-1] + if not rest: + return m.group(0) + cut = len(rest) + for ch in ('/', '?', '#'): + idx = rest.find(ch) + if idx != -1: + cut = min(cut, idx) + authority, tail = rest[:cut], rest[cut:] + changed = False + at = authority.rfind('@') + if at != -1: + userinfo = authority[:at] + user, colon, password = userinfo.partition(':') + if colon and password and _looks_real(password, quoted=False): + authority = (f'{user}:{_marker("password")}@' + + authority[at + 1:]) + changed = True + hits.append(('password', _line_of(text, + m.start()), 'userinfo')) + elif not colon and userinfo and _looks_real( + userinfo, quoted=False): + # Colon-less userinfo is how PAT-style tokens travel + # (``https://ghp_xxx@host``) and cannot be told from a + # username: fail closed, as ``scrub_url_secrets`` does. + authority = f'{_marker("token")}@' + authority[at + 1:] + changed = True + hits.append(('token', _line_of(text, m.start()), '')) + if '?' in tail: + path, query = tail.split('?', 1) + fragment = '' + if '#' in query: + query, frag = query.split('#', 1) + fragment = '#' + frag + pairs = [] + for pair in query.split('&'): + name, eq, value = pair.partition('=') + strength = name_strength(name) if eq else _DENY + if strength > _DENY and _looks_real( + value, quoted=True, strength=strength): + pairs.append(f'{name}={_marker("api_key")}') + changed = True + hits.append(('api_key', _line_of(text, m.start()), name)) + else: + pairs.append(pair) + tail = path + '?' + '&'.join(pairs) + fragment + if not changed: + return m.group(0) + return f'{m.group("scheme")}{authority}{tail}{trail}' + + return _FREE_URL_RE.sub(repl, text) + + +def _redact_assignments(text: str, hits: list) -> str: + + def repl(m): + value = m.group('val') + quoted = m.group('vq3') in ('"', "'") + strength = name_strength(m.group('name')) + if strength == _DENY: + return m.group(0) + if not _looks_real(value, quoted=quoted, strength=strength): + return m.group(0) + hits.append(('credential', _line_of(text, + m.start('val')), m.group('name'))) + return (f"{m.group('vq1')}{m.group('name')}{m.group('vq2')}" + f"{m.group('sep')}{m.group('vq3')}{_marker('credential')}" + f"{m.group('vq3')}") + + return _ASSIGN_RE.sub(repl, text) + + +def _redact_flags(text: str, hits: list) -> str: + + def repl(m): + value = m.group('val') + if value is None: + return m.group(0) + quoted = m.group('vq') in ('"', "'") + strength = name_strength(m.group('name')) + if strength == _DENY: + return m.group(0) + if not _looks_real(value, quoted=quoted, strength=strength): + return m.group(0) + hits.append(('flag', _line_of(text, m.start('val')), m.group('name'))) + return (f"--{m.group('name')}={m.group('vq')}{_marker('flag')}" + f"{m.group('vq')}") + + return _FLAG_RE.sub(repl, text) + + +def _redact_base64(text: str, hits: list) -> str: + """Replace a base64 payload whose DECODED content is a secret. + + Persona files sometimes carry "decode this at runtime" instructions whose + plaintext never appears in the file. Only blobs that decode to valid + printable text matching a credential rule are touched, so hashes (invalid + UTF-8), data URIs and ordinary identifiers survive. + """ + attempts = 0 + + def repl(m): + nonlocal attempts + blob = m.group('blob') + start = m.start('blob') + if attempts >= _BASE64_MAX_ATTEMPTS: + return m.group(0) + attempts += 1 + if text[max(0, start - 5):start].endswith('data:'): + return m.group(0) + try: + decoded = base64.b64decode(blob, validate=True) + except (binascii.Error, ValueError): + return m.group(0) + try: + plain = decoded.decode('utf-8') + except UnicodeDecodeError: + return m.group(0) + if not plain or any(c in plain for c in '\x00\n\r\t'): + return m.group(0) + inner: list = [] + scanned = _redact_vendor(plain, inner) + scanned = _redact_jwt(scanned, inner) + if not inner: + assign_hits: list = [] + scanned = _redact_assignments(plain, assign_hits) + if not assign_hits: + return m.group(0) + inner = assign_hits + hits.append(('base64', _line_of(text, start), inner[0][2])) + return _marker('base64') + + return _BASE64_RE.sub(repl, text) + + +# One cheap union of every Tier B textual trigger. A file that matches none of +# these cannot be redacted by any of the gated passes, so skipping them is +# lossless -- and the watch daemon re-runs this over the whole workspace every +# poll. Deliberately over-matches (``monkey`` contains ``key``): it is a gate, +# not a rule. The base64 pass is NOT gated, because an encoded payload carries +# no textual signal at all. +_PREFILTER_RE = re.compile( + r'(?:sk-|gh[pousr]_|github_pat_|glpat-|xox[baprs]-|AKIA|AIza|hf_|npm_|' + r'shpat_|eyJ|[Bb]earer|://|' + r'key|keys|token|tokens|secret|secrets|password|passwd|credential|' + r'credentials|authorization|cookie|cookies)', re.IGNORECASE) + + +def redact_text(text: str) -> tuple[str, tuple[tuple[str, int, str], ...]]: + """Redact high-confidence secret values in free text (Tier B). + + Returns ``(text, hits)`` where each hit is ``(kind, line, name)``. The + input is returned unchanged (the same ``str`` object) when nothing + matched. Passes run in a fixed order -- base64, vendor prefixes, JWT, + ``Bearer``, URLs, ``NAME=VALUE``, ``--flag VALUE`` -- so a secret is + replaced by the most specific rule that sees it first and the later passes + only ever encounter a ``[REDACTED:...]`` marker, which no rule matches. + """ + if not text: + return text, () + hits: list[tuple[str, int, str]] = [] + out = _redact_base64(text, hits) + if _PREFILTER_RE.search(text): + out = _redact_vendor(out, hits) + out = _redact_jwt(out, hits) + out = _redact_bearer(out, hits) + out = _redact_urls(out, hits) + out = _redact_assignments(out, hits) + out = _redact_flags(out, hits) + if out == text: + return text, () + return out, tuple(hits) + + +# --------------------------------------------------------------------------- +# Tier A: config-shaped files at ANY path +# --------------------------------------------------------------------------- + +_JSON_CONFIG_NAMES = frozenset(('mcp.json', 'settings.json', 'agent.json')) +_YAML_CONFIG_NAMES = frozenset( + ('config.yaml', 'config.yml', 'mcp.yaml', 'mcp.yml')) +_TOML_CONFIG_NAMES = frozenset(('config.toml', 'mcp.toml')) +_MCP_JSON_SUFFIXES = ('.mcp.json', ) + +_MCP_ROOT_KEYS = frozenset(('mcpServers', 'mcp_servers', 'mcpservers')) + +_YAML_MCP_SHAPE_RE = re.compile( + r'^[ \t]*["\']?(?:mcpServers|mcp_servers)' + r'["\']?[ \t]*:', re.MULTILINE) +_TOML_MCP_SHAPE_RE = re.compile( + r'(?:^[ \t]*\[{1,2}[^\]\n]*\b(?:mcpServers|mcp_servers)\b' + r'|^[ \t]*(?:mcpServers|mcp_servers)[ \t]*=)', re.MULTILINE) + + +def _json_has_mcp(obj) -> bool: + if isinstance(obj, dict): + for key, val in obj.items(): + if key in _MCP_ROOT_KEYS and isinstance(val, dict): + return True + if _json_has_mcp(val): + return True + elif isinstance(obj, list): + return any(_json_has_mcp(item) for item in obj) + return False + + +def _scrub_json_mcp_subtrees(obj) -> bool: + """Clean only the ``mcpServers`` subtrees of a parsed JSON document. + + A skill's ``mcp.json``-shaped payload embedded in an otherwise unrelated + JSON document gets the full structural policy (``env`` / ``headers`` bags + cleared, ``args`` command lines scrubbed, URL credentials stripped) while + the rest of the document -- which may legitimately hold ``tokens`` or + ``keys`` data fields -- is left alone. Returns whether anything changed. + """ + changed = False + if isinstance(obj, dict): + for key, val in obj.items(): + if key in _MCP_ROOT_KEYS and isinstance(val, dict): + before = json.dumps(val, sort_keys=True, ensure_ascii=False) + scrub_json_secrets(val) + if json.dumps( + val, sort_keys=True, ensure_ascii=False) != before: + changed = True + elif _scrub_json_mcp_subtrees(val): + changed = True + elif isinstance(obj, list): + for item in obj: + if _scrub_json_mcp_subtrees(item): + changed = True + return changed + + +def _redact_config(rel_path: str, text: str, hits: list[tuple[str, int, + str]]) -> str: + """Tier A: structural cleaning of a config-shaped file at any path. + + Best effort by design. A known config name that fails to parse falls + through to Tier B instead of raising: refusing to upload a skill's JSON + data fixture would be worse than redacting the secret values inside it. + (The per-framework hook already fails closed for the ROOT config files it + owns, and it runs before this layer.) + """ + base = rel_path.rsplit('/', 1)[-1].lower() + if base.endswith('.json') or base.endswith('.jsonl'): + is_known = base in _JSON_CONFIG_NAMES or base.endswith( + _MCP_JSON_SUFFIXES) + try: + data = json.loads(text) + except (ValueError, RecursionError): + return text + if is_known: + before = json.dumps(data, sort_keys=True, ensure_ascii=False) + scrub_json_secrets(data) + if json.dumps(data, sort_keys=True, ensure_ascii=False) == before: + return text + hits.append(('config', 0, base)) + return json.dumps(data, ensure_ascii=False, indent=2) + if _json_has_mcp(data) and _scrub_json_mcp_subtrees(data): + hits.append(('config', 0, base)) + return json.dumps(data, ensure_ascii=False, indent=2) + return text + if base.endswith(('.yaml', '.yml')): + if base not in _YAML_CONFIG_NAMES \ + and not _YAML_MCP_SHAPE_RE.search(text): + return text + cleaned = scrub_yaml_secrets(text) + if cleaned != text: + hits.append(('config', 0, base)) + return cleaned + if base.endswith('.toml'): + if base not in _TOML_CONFIG_NAMES \ + and not _TOML_MCP_SHAPE_RE.search(text): + return text + cleaned = scrub_toml_secrets(text) + if cleaned != text: + hits.append(('config', 0, base)) + return cleaned + return text + + +# --------------------------------------------------------------------------- +# entry point +# --------------------------------------------------------------------------- + +# Bounded sha256 -> result memo. The watcher re-sanitizes the whole workspace +# every poll and upload re-runs it per invocation, so identical content is +# scanned once. Capped on both entry count and per-file size to keep the +# daemon's footprint flat. +_MEMO_MAX_ENTRIES = 128 +_MEMO_MAX_FILE_SIZE = 128 * 1024 +_memo: 'OrderedDict[tuple[str, str], tuple[bytes, tuple[Finding, ...]]]' = \ + OrderedDict() + + +def _redact(rel_path: str, raw: bytes) -> tuple[bytes, tuple[Finding, ...]]: + try: + text = raw.decode('utf-8') + except UnicodeDecodeError: + # Binary asset (image, PDF, archive): not a text-secret carrier. + return raw, () + + hits: list[tuple[str, int, str]] = [] + out = _redact_config(rel_path, text, hits) + out, text_hits = redact_text(out) + hits.extend(text_hits) + if not hits: + # Byte identity matters: see the module contract. + return raw, () + return out.encode('utf-8'), tuple( + Finding(rel_path, kind, line, name) for kind, line, name in hits) + + +def redact_outbound(rel_path: str, + raw: bytes) -> tuple[bytes, tuple[Finding, ...]]: + """Redact secrets in one collected file, by content rather than by path. + + Runs after the framework's own :meth:`sanitize_outbound_file` hook. Never + raises: an unparseable, oversized or exotic file is returned unchanged + (Tier B still gets a best-effort pass at its text), because a crashed + sanitize would either block the upload or -- inside the watch daemon, + which swallows exceptions -- silently stop syncing. + """ + try: + digest = hashlib.sha256(raw).hexdigest() + memo_key = (rel_path, digest) + cached = _memo.get(memo_key) + if cached is not None: + _memo.move_to_end(memo_key) + cleaned, hits = cached + # The cached bytes came from an earlier call, so hand back THIS + # caller's object when nothing was redacted: the contract is to + # return the original, not merely an equal copy. + return (raw if cleaned == raw else cleaned), hits + result = _redact(rel_path, raw) + if len(raw) <= _MEMO_MAX_FILE_SIZE: + _memo[memo_key] = result + while len(_memo) > _MEMO_MAX_ENTRIES: + _memo.popitem(last=False) + return result + except Exception: + logger.debug( + 'Secret redaction failed for %s; uploading as-is.', + rel_path, + exc_info=True) + return raw, () diff --git a/ms_agent/agent_hub/_sync.py b/ms_agent/agent_hub/_sync.py index 85d59c9a0..edfb5d3ad 100644 --- a/ms_agent/agent_hub/_sync.py +++ b/ms_agent/agent_hub/_sync.py @@ -13,6 +13,7 @@ from ms_agent.utils.logger import get_logger from ._cache import cache_dir from ._defaults import get_defaults +from ._secrets import redact_outbound if TYPE_CHECKING: from modelscope_hub.agent import AgentApi, RemoteFileInfo @@ -75,24 +76,45 @@ def backup_local(spec, name: str) -> Path: return zip_path -def sanitize_outbound(resources: dict, spec) -> dict: +def sanitize_outbound(resources: dict, + spec, + findings: list | None = None) -> dict: """Strip machine-local secrets from local files before they are pushed. - Symmetric to the per-file inbound sanitize hook: applies - ``spec.sanitize_outbound_file`` to every collected file so secrets a user - left in a local config (e.g. hermes ``config.yaml``, openhuman - ``config.toml``, qwenpaw ``agent.json``) are never uploaded to the remote - repo -- and therefore never written into its git history. + Two layers, both applied to EVERY collected file: + + 1. ``spec.sanitize_outbound_file`` -- the framework's own hook. It owns the + structural cleaning of the config files it knows about (hermes + ``config.yaml``, openhuman ``config.toml``, qwenpaw ``agent.json``, ...) + and fails closed (raises ``ValueError``) on one it cannot parse. + 2. :func:`._secrets.redact_outbound` -- content-driven and + path-independent. The hook selects files by PATH, so a key an AI + assistant left in ``skills/*/scripts/*.py``, a skill-local ``mcp.json``, + ``SOUL.md`` or ``MEMORY.md`` used to be uploaded verbatim into the + remote repo and its git history (BUG-0909-01). This layer decides on + content instead, which also covers the frameworks that define no hook at + all (openclaw, nanobot, qoder). + + *findings*, when given, receives one :class:`._secrets.Finding` per + redacted secret (never the secret text itself) so the caller can tell the + user what was stripped. Accepts and returns the same ``{rel_path: bytes}`` mapping ``collect_bytes`` produces (``str`` values are encoded as UTF-8, matching - the push path which uploads bytes). + the push path which uploads bytes). A file with nothing to redact keeps its + ORIGINAL bytes object: ``drop_unchanged_defaults`` compares bytes against + the framework default templates and ``push_mirror`` skips uploads by + sha256, so re-serializing an untouched file would defeat both. """ out: dict = {} for rel, content in resources.items(): raw = content if isinstance(content, bytes) else content.encode('utf-8') - out[rel] = spec.sanitize_outbound_file(rel, raw) + cleaned = spec.sanitize_outbound_file(rel, raw) + cleaned, hits = redact_outbound(rel, cleaned) + if findings is not None: + findings.extend(hits) + out[rel] = cleaned return out diff --git a/ms_agent/agent_hub/_watcher.py b/ms_agent/agent_hub/_watcher.py index 84d4345df..7a1995680 100644 --- a/ms_agent/agent_hub/_watcher.py +++ b/ms_agent/agent_hub/_watcher.py @@ -154,8 +154,15 @@ def _poll_once(client, username, repo, framework, spec, push_only, state, # Sanitize first so a secret left in a local config is never pushed, and # never enters the sync baseline as pushable content (which would # otherwise re-push it every cycle). + redacted: list = [] local_resources = drop_unchanged_defaults( - sanitize_outbound(spec.collect_bytes(), spec), framework, spec) + sanitize_outbound(spec.collect_bytes(), spec, findings=redacted), + framework, spec) + if redacted: + # A Finding never carries the secret text, so this is safe to log. + where = ', '.join(sorted({f.rel for f in redacted})) + logger.warning('Redacted %d secret value(s) before push, in: %s', + len(redacted), where) baseline = state.get('remote_files', {}) # A remote file counts toward change detection if it was in our baseline # (so edits/deletions are seen) OR it is a workspace file the collect diff --git a/ms_agent/agent_hub/_workspace.py b/ms_agent/agent_hub/_workspace.py index cd2d07322..cc3e0179c 100644 --- a/ms_agent/agent_hub/_workspace.py +++ b/ms_agent/agent_hub/_workspace.py @@ -550,6 +550,161 @@ def scrub_json_secrets(obj) -> None: scrub_json_secrets(item) +def _blank_toml_value(out: list[str], lines: list[str], i: int, pre: str, + val: str) -> int: + """Blank the value of the TOML assignment at ``lines[i]``, appending to *out*. + + Handles multi-line strings (``'''`` / ``\"\"\"``), (multi-line) arrays and + plain scalars; returns the next line index to process. + """ + vstrip = val.strip() + delim = next((d for d in ('"""', "'''") + if vstrip.startswith(d) and vstrip.count(d) < 2), None) + if delim: + out.append(pre + '""') + i += 1 + while i < len(lines) and delim not in lines[i]: + i += 1 + return i + 1 + if vstrip.startswith('['): + out.append(pre + '[]') + if ']' not in vstrip: + i += 1 + while i < len(lines) and ']' not in lines[i]: + i += 1 + return i + 1 + out.append(pre + '""') + return i + 1 + + +def scrub_toml_secrets(text: str) -> str: + """Blank secret values in a TOML config *text*, line-by-line. + + Line-level rewrite (stdlib has no TOML writer): any ``key = `` + assignment whose key name matches :func:`is_secret_key` has its value + cleared to ``""``, preserving the rest of the file verbatim. + + Dotted keys (``model.api_key = ...``) are allowed and only the LAST + segment is tested, so ``a.b.api_key`` is caught, not just a bare + top-level ``api_key``. Beyond simple ``key = `` lines this also + covers: + + * inline tables ``provider = { api_key = "X" }`` (incl. nested) -- + secret pairs inside the braces are cleared in place; an + ``env`` / ``headers`` inline table (:data:`SECRET_BAG_KEYS`) is cleared + wholesale -- those names are arbitrary bearer bags; + * table sections ``[mcp.fs.headers]`` / ``[[...env]]`` -- when the + LAST path segment is a secret bag every assignment inside the + section is blanked, including quoted keys (``"X-Auth-Code"``) + that the bare-key pattern cannot match; + * arrays ``tokens = ["X"]`` -> ``tokens = []``; an ``args`` + array (single- or multi-line) is positionally scrubbed (secret + flag values blanked); a clean multi-line array keeps its layout; + * multi-line strings (``secret = '''`` / ``\"\"\"``) -- the opener is + blanked and the content lines up to the closing delimiter dropped; + * URL string values under any key -- userinfo passwords and + secret-named query parameters are stripped. + + Shared by openhuman's ``config.toml`` and by the content-driven outbound + layer (:mod:`ms_agent.agent_hub._secrets`), which cleans ``.toml`` at ANY + collected path -- one secret vocabulary for both. + """ + pattern = re.compile( + r'^(?P
\s*(?P[A-Za-z0-9_.-]+)\s*=\s*)(?P.*)$')
+    inline_pair = re.compile(r'(?P[A-Za-z0-9_.-]+)(?P\s*=\s*)'
+                             r'(?P"[^"]*"|\'[^\']*\'|[^,{}\s][^,}]*)')
+    section = re.compile(r'^\s*\[{1,2}\s*(?P[^#\]\[]+?)\s*\]{1,2}'
+                         r'\s*(?:#.*)?$')
+    bagged_assign = re.compile(r'^(?P
\s*.+?=\s*)(?P.*)$')
+    out: list[str] = []
+    lines = text.split('\n')
+    # True while inside a ``[...headers]`` / ``[...env]`` table section.
+    in_bag_section = False
+    i = 0
+    while i < len(lines):
+        line = lines[i]
+        sm = section.match(line)
+        if sm:
+            seg = sm.group('path').split('.')[-1].strip().strip('"\'')
+            in_bag_section = seg in SECRET_BAG_KEYS
+            out.append(line)
+            i += 1
+            continue
+        m = pattern.match(line)
+        if not m:
+            # Quoted keys (``"X-Auth-Code" = ...``) fail the bare-key
+            # pattern; inside a bag section they must still be blanked.
+            if in_bag_section and not line.strip().startswith('#'):
+                bm = bagged_assign.match(line)
+                if bm:
+                    i = _blank_toml_value(out, lines, i, bm.group('pre'),
+                                          bm.group('val'))
+                    continue
+            out.append(line)
+            i += 1
+            continue
+        segs = [s.strip().strip('"\'') for s in m.group('key').split('.')]
+        key = segs[-1]
+        val = m.group('val')
+        vstrip = val.strip()
+        # Dotted path THROUGH a bag (``mcp.fs.headers.X = v``) is the
+        # same as living in a ``[...headers]`` section.
+        in_bag_path = any(s in SECRET_BAG_KEYS for s in segs[:-1])
+        if is_secret_key(key) or in_bag_section or in_bag_path:
+            i = _blank_toml_value(out, lines, i, m.group('pre'), val)
+            continue
+        # Secret-bag inline table (``headers = {...}`` / ``env = {...}``):
+        # clear every pair -- the names inside are arbitrary.
+        if key in SECRET_BAG_KEYS and vstrip.startswith('{'):
+            out.append(
+                m.group('pre') + inline_pair.sub(
+                    lambda pm: f"{pm.group('key')}{pm.group('sep')}\"\"", val))
+            i += 1
+            continue
+        # ``args = [...]``: positional flag scrub. A multi-line array is
+        # joined for the scrub and only re-emitted on one line when a
+        # secret was actually removed (clean arrays keep their layout).
+        if key in ARGS_LIST_KEYS and vstrip.startswith('['):
+            if ']' in vstrip:
+                out.append(m.group('pre') + scrub_toml_array_args(val))
+                i += 1
+                continue
+            j = i + 1
+            parts = [val.strip()]
+            while j < len(lines) and ']' not in lines[j]:
+                parts.append(lines[j].strip())
+                j += 1
+            if j < len(lines):
+                parts.append(lines[j].strip())
+                joined = ' '.join(parts)
+                cleaned = scrub_toml_array_args(joined)
+                if cleaned == joined:
+                    out.extend(lines[i:j + 1])
+                else:
+                    out.append(m.group('pre') + cleaned.strip())
+                i = j + 1
+                continue
+        # Non-secret key: still scrub secret pairs inside inline tables
+        # (``provider = { api_key = "X" }``, nested tables included), and
+        # strip credentials from URL string values.
+        if '{' in vstrip:
+
+            def _repl(pm):
+                if is_secret_key(pm.group('key').split('.')[-1]):
+                    return f"{pm.group('key')}{pm.group('sep')}\"\""
+                cleaned = scrub_scalar_url_token(pm.group('val'))
+                if cleaned != pm.group('val'):
+                    return f"{pm.group('key')}{pm.group('sep')}{cleaned}"
+                return pm.group(0)
+
+            out.append(m.group('pre') + inline_pair.sub(_repl, val))
+            i += 1
+            continue
+        out.append(m.group('pre') + scrub_scalar_url_token(val))
+        i += 1
+    return '\n'.join(out)
+
+
 class WorkspaceSpec(ABC):
     """Abstract base for agent framework workspace file specifications.
 
@@ -864,6 +1019,14 @@ def sanitize_outbound_file(self, rel_path: str, content: bytes) -> bytes:
         outbound sanitize for free. Frameworks whose inbound hook DOES rebind
         identity (qwenpaw ``agent.json``) must override this to blank secrets
         WITHOUT writing machine-local identity into the upload.
+
+        This hook is only the FIRST of two outbound layers, and it selects files
+        by PATH. :func:`._sync.sanitize_outbound` then runs every file through
+        the content-driven :func:`._secrets.redact_outbound`, which catches
+        secrets in the files no framework whitelists (``skills/*``, persona and
+        memory documents) -- so a framework that defines no hook at all is still
+        covered. Keep this hook for the structural cleaning and the fail-closed
+        refusals of the config files a framework owns.
         """
         return self.sanitize_inbound_file(rel_path, content)
 
diff --git a/ms_agent/agent_hub/frameworks/openhuman.py b/ms_agent/agent_hub/frameworks/openhuman.py
index 5750dee22..bd5bfd5aa 100644
--- a/ms_agent/agent_hub/frameworks/openhuman.py
+++ b/ms_agent/agent_hub/frameworks/openhuman.py
@@ -4,13 +4,11 @@
 
 import copy
 import json
-import re
 from pathlib import Path
 
 from ms_agent.utils.logger import get_logger
-from .._workspace import (ARGS_LIST_KEYS, DEFAULT_AGENT_NAME, SECRET_BAG_KEYS,
-                          WorkspaceSpec, is_secret_key, register_framework,
-                          scrub_scalar_url_token, scrub_toml_array_args)
+from .._workspace import (DEFAULT_AGENT_NAME, WorkspaceSpec,
+                          register_framework, scrub_toml_secrets)
 
 logger = get_logger()
 
@@ -313,144 +311,14 @@ def sanitize_inbound_file(self, rel_path: str, content: bytes) -> bytes:
         return self._scrub_toml_secrets(text).encode('utf-8')
 
     def _scrub_toml_secrets(self, text: str) -> str:
-        # Allow dotted keys (``model.api_key = ...``) and test the last segment
-        # so ``a.b.api_key`` is caught, not just a bare top-level ``api_key``.
-        # Beyond simple ``key = `` lines this also covers:
-        # * inline tables  ``provider = { api_key = "X" }`` (incl. nested) --
-        #   secret pairs inside the braces are cleared in place; an
-        #   ``env`` / ``headers`` inline table (SECRET_BAG_KEYS) is cleared
-        #   wholesale -- those names are arbitrary bearer bags;
-        # * table sections ``[mcp.fs.headers]`` / ``[[...env]]`` -- when the
-        #   LAST path segment is a secret bag every assignment inside the
-        #   section is blanked, including quoted keys (``"X-Auth-Code"``)
-        #   that the bare-key pattern cannot match;
-        # * arrays         ``tokens = ["X"]`` -> ``tokens = []``; an ``args``
-        #   array (single- or multi-line) is positionally scrubbed (secret
-        #   flag values blanked); a clean multi-line array keeps its layout;
-        # * multi-line strings (``secret = '''`` / ``\"\"\"``) -- the opener is
-        #   blanked and the content lines up to the closing delimiter dropped;
-        # * URL string values under any key -- userinfo passwords and
-        #   secret-named query parameters are stripped.
-        pattern = re.compile(
-            r'^(?P
\s*(?P[A-Za-z0-9_.-]+)\s*=\s*)(?P.*)$')
-        inline_pair = re.compile(r'(?P[A-Za-z0-9_.-]+)(?P\s*=\s*)'
-                                 r'(?P"[^"]*"|\'[^\']*\'|[^,{}\s][^,}]*)')
-        section = re.compile(r'^\s*\[{1,2}\s*(?P[^#\]\[]+?)\s*\]{1,2}'
-                             r'\s*(?:#.*)?$')
-        bagged_assign = re.compile(r'^(?P
\s*.+?=\s*)(?P.*)$')
-        out: list[str] = []
-        lines = text.split('\n')
-        # True while inside a ``[...headers]`` / ``[...env]`` table section.
-        in_bag_section = False
-        i = 0
-        while i < len(lines):
-            line = lines[i]
-            sm = section.match(line)
-            if sm:
-                seg = sm.group('path').split('.')[-1].strip().strip('"\'')
-                in_bag_section = seg in SECRET_BAG_KEYS
-                out.append(line)
-                i += 1
-                continue
-            m = pattern.match(line)
-            if not m:
-                # Quoted keys (``"X-Auth-Code" = ...``) fail the bare-key
-                # pattern; inside a bag section they must still be blanked.
-                if in_bag_section and not line.strip().startswith('#'):
-                    bm = bagged_assign.match(line)
-                    if bm:
-                        i = self._blank_toml_value(
-                            out, lines, i, bm.group('pre'), bm.group('val'))
-                        continue
-                out.append(line)
-                i += 1
-                continue
-            segs = [s.strip().strip('"\'')
-                    for s in m.group('key').split('.')]
-            key = segs[-1]
-            val = m.group('val')
-            vstrip = val.strip()
-            # Dotted path THROUGH a bag (``mcp.fs.headers.X = v``) is the
-            # same as living in a ``[...headers]`` section.
-            in_bag_path = any(s in SECRET_BAG_KEYS for s in segs[:-1])
-            if is_secret_key(key) or in_bag_section or in_bag_path:
-                i = self._blank_toml_value(out, lines, i, m.group('pre'), val)
-                continue
-            # Secret-bag inline table (``headers = {...}`` / ``env = {...}``):
-            # clear every pair -- the names inside are arbitrary.
-            if key in SECRET_BAG_KEYS and vstrip.startswith('{'):
-                out.append(
-                    m.group('pre') + inline_pair.sub(
-                        lambda pm: f"{pm.group('key')}{pm.group('sep')}\"\"",
-                        val))
-                i += 1
-                continue
-            # ``args = [...]``: positional flag scrub. A multi-line array is
-            # joined for the scrub and only re-emitted on one line when a
-            # secret was actually removed (clean arrays keep their layout).
-            if key in ARGS_LIST_KEYS and vstrip.startswith('['):
-                if ']' in vstrip:
-                    out.append(m.group('pre') + scrub_toml_array_args(val))
-                    i += 1
-                    continue
-                j = i + 1
-                parts = [val.strip()]
-                while j < len(lines) and ']' not in lines[j]:
-                    parts.append(lines[j].strip())
-                    j += 1
-                if j < len(lines):
-                    parts.append(lines[j].strip())
-                    joined = ' '.join(parts)
-                    cleaned = scrub_toml_array_args(joined)
-                    if cleaned == joined:
-                        out.extend(lines[i:j + 1])
-                    else:
-                        out.append(m.group('pre') + cleaned.strip())
-                    i = j + 1
-                    continue
-            # Non-secret key: still scrub secret pairs inside inline tables
-            # (``provider = { api_key = "X" }``, nested tables included), and
-            # strip credentials from URL string values.
-            if '{' in vstrip:
-                def _repl(pm):
-                    if is_secret_key(pm.group('key').split('.')[-1]):
-                        return f"{pm.group('key')}{pm.group('sep')}\"\""
-                    cleaned = scrub_scalar_url_token(pm.group('val'))
-                    if cleaned != pm.group('val'):
-                        return f"{pm.group('key')}{pm.group('sep')}{cleaned}"
-                    return pm.group(0)
-
-                out.append(m.group('pre') + inline_pair.sub(_repl, val))
-                i += 1
-                continue
-            out.append(m.group('pre') + scrub_scalar_url_token(val))
-            i += 1
-        return '\n'.join(out)
-
-    @staticmethod
-    def _blank_toml_value(out: list[str], lines: list[str], i: int,
-                            pre: str, val: str) -> int:
-        """Blank the value of the assignment at ``lines[i]``, appending to
-        *out*. Handles multi-line strings (``'''`` / ``\"\"\"``), (multi-line)
-        arrays and plain scalars; returns the next line index to process."""
-        vstrip = val.strip()
-        delim = next((d for d in ('"""', "'''")
-                      if vstrip.startswith(d) and vstrip.count(d) < 2), None)
-        if delim:
-            out.append(pre + '""')
-            i += 1
-            while i < len(lines) and delim not in lines[i]:
-                i += 1
-            return i + 1
-        if vstrip.startswith('['):
-            out.append(pre + '[]')
-            if ']' not in vstrip:
-                i += 1
-                while i < len(lines) and ']' not in lines[i]:
-                    i += 1
-            return i + 1
-        out.append(pre + '""')
-        return i + 1
+        """Thin wrapper over the shared :func:`scrub_toml_secrets`.
+
+        The implementation moved to ``_workspace`` so the content-driven
+        outbound layer can clean ``.toml`` at ANY collected path with the same
+        rules (dotted keys, inline tables, ``[...env]`` / ``[...headers]``
+        sections, ``args`` arrays, multi-line strings, URL scalars).
+        """
+        return scrub_toml_secrets(text)
 
 
 register_framework('openhuman', OpenhumanWorkspace)
diff --git a/tests/agent_hub/test_cli.py b/tests/agent_hub/test_cli.py
index d2295ba67..c20809826 100644
--- a/tests/agent_hub/test_cli.py
+++ b/tests/agent_hub/test_cli.py
@@ -1,5 +1,7 @@
 # Copyright (c) ModelScope Contributors. All rights reserved.
 """CLI command tests: helper functions, upload/download/convert flows (stubbed client)."""
+import base64
+import contextlib
 import io
 import os
 import tempfile
@@ -1765,6 +1767,113 @@ def test_upload_scrubs_ms_agent_settings_json_secrets(self):
         self.assertNotIn("sk-LEAKME333", raw)
         self.assertNotIn("env-LEAKME444", raw)
 
+    # BUG-0909-01: the framework hooks select files by PATH, so everything
+    # outside the root config whitelist (``skills/*``, the persona and memory
+    # documents) used to be uploaded verbatim.
+    B64_PERSONA_KEY = base64.b64encode(b"sk-S28PersonaB64Leak1").decode()
+    SKILL_SENTINELS = (
+        "sk-SkillScriptLeak01",
+        "ghp_SkillPat7x9Qm2Rt4V",
+        "S32SkillEnvLeak0001",
+        "S33SkillUrlLeak0001",
+        "S34SkillDocLeak0001",
+        "sk-S26PersonaProse01",
+        "sk-S27PersonaBearer1",
+        B64_PERSONA_KEY,
+    )
+
+    def _skill_tree_files(self):
+        import json
+        return {
+            "SOUL.md": (
+                "# Persona\n\n"
+                "You are a helpful weather assistant.\n\n"
+                "调用 API 时使用 sk-S26PersonaProse01 作为密钥。\n\n"
+                "```bash\n"
+                'curl -H "Authorization: Bearer sk-S27PersonaBearer1" '
+                "https://api.example.com/v1\n"
+                "```\n\n"
+                f'    echo "{self.B64_PERSONA_KEY}" | base64 -d\n\n'
+                "Benign lines that must survive:\n\n"
+                "- Use `os.environ[\"DASHSCOPE_API_KEY\"]` to read the key.\n"
+                "- Pass `--api-key ` on the command line.\n"),
+            "skills/weather/scripts/leak_point.py": (
+                '"""Fetch the forecast."""\n'
+                "import os\n\n"
+                'API_KEY = "sk-SkillScriptLeak01"\n'
+                'GITHUB_TOKEN = "ghp_SkillPat7x9Qm2Rt4V"\n\n\n'
+                "def fetch(city):\n"
+                '    return city, os.environ.get("OPENWEATHER_KEY", "")\n'),
+            "skills/weather/mcp.json": json.dumps({
+                "mcpServers": {
+                    "weather": {
+                        "command": "uvx",
+                        "args": ["-y", "mcp-server-weather"],
+                        "env": {"OPENWEATHER_KEY": "S32SkillEnvLeak0001"},
+                        "url": ("https://api.example.com/v1"
+                                "?api_key=S33SkillUrlLeak0001"),
+                    }
+                }
+            }),
+            "skills/weather/SKILL.md": (
+                "---\nname: weather\n---\n\n"
+                "```json\n"
+                '{"mcpServers": {"weather": '
+                '{"env": {"OPENWEATHER_KEY": "S34SkillDocLeak0001"}}}}\n'
+                "```\n\n"
+                "```bash\n"
+                'curl -H "Authorization: Bearer $OPENAI_API_KEY" https://h/v1\n'
+                "```\n"),
+        }
+
+    @mock.patch("ms_agent.agent_hub._commands.AgentApi", _StubClient)
+    def test_upload_scrubs_skill_tree_and_persona(self):
+        root = self._write_ws("ms-agent", self._skill_tree_files())
+        buf = io.StringIO()
+        with contextlib.redirect_stdout(buf):
+            rc = cmd_upload(
+                framework="ms-agent", name=None, local_dir=str(root),
+                endpoint="http://s", token="tok", username="u",
+            )
+        self.assertEqual(rc, 0)
+        client = _StubClient.instances[0]
+        # The skill tree really was collected -- otherwise the assertions
+        # below would pass vacuously.
+        self.assertIn("skills/weather/scripts/leak_point.py",
+                      client.uploaded_resources)
+        self.assertIn("skills/weather/mcp.json", client.uploaded_resources)
+        blob = "\n".join(v.decode("utf-8", "replace")
+                         for v in client.uploaded_resources.values())
+        for sentinel in self.SKILL_SENTINELS:
+            self.assertNotIn(sentinel, blob)
+        # Benign documentation survives: the gate must not eat placeholders.
+        self.assertIn('os.environ.get("OPENWEATHER_KEY", "")', blob)
+        self.assertIn('os.environ["DASHSCOPE_API_KEY"]', blob)
+        self.assertIn("--api-key ", blob)
+        self.assertIn("Bearer $OPENAI_API_KEY", blob)
+        self.assertIn("You are a helpful weather assistant.", blob)
+        # The user is told what was stripped -- without echoing the secret.
+        report = buf.getvalue()
+        self.assertIn("Secrets redacted", report)
+        self.assertIn("skills/weather/scripts/leak_point.py", report)
+        for sentinel in self.SKILL_SENTINELS:
+            self.assertNotIn(sentinel, report)
+
+    @mock.patch("ms_agent.agent_hub._commands.AgentApi", _StubClient)
+    def test_dry_run_reports_redactions_without_uploading(self):
+        root = self._write_ws("ms-agent", self._skill_tree_files())
+        buf = io.StringIO()
+        with contextlib.redirect_stdout(buf):
+            rc = cmd_upload(
+                framework="ms-agent", name=None, local_dir=str(root),
+                dry_run=True, endpoint="http://s", token="tok", username="u",
+            )
+        self.assertEqual(rc, 0)
+        report = buf.getvalue()
+        self.assertIn("[dry-run] nothing uploaded.", report)
+        self.assertIn("Secrets redacted", report)
+        self.assertFalse(_StubClient.instances)
+
 
 class _OpenclawStub(_RepoStub):
     """Serves an openclaw single sub-agent repo (bare paths)."""
diff --git a/tests/agent_hub/test_secrets.py b/tests/agent_hub/test_secrets.py
new file mode 100644
index 000000000..bbc5462fb
--- /dev/null
+++ b/tests/agent_hub/test_secrets.py
@@ -0,0 +1,466 @@
+# Copyright (c) Alibaba, Inc. and its affiliates.
+"""Content-driven outbound secret redaction tests (BUG-0909-01).
+
+The framework ``sanitize_outbound_file`` hooks select files by PATH, so a key
+left in ``skills/*``, a persona document or a memory file used to be uploaded
+verbatim. These tests pin the replacement layer: what it must catch, what it
+must leave alone (documentation is full of secret-shaped placeholders), and the
+byte-identity / idempotency contract the sync paths depend on.
+"""
+import base64
+import json
+import unittest
+
+from ms_agent.agent_hub._secrets import (Finding, name_strength,
+                                         redact_outbound, redact_text)
+
+SK_KEY = "sk-S26ProseLeak0001"
+GH_PAT = "ghp_S25bPatLeak7x9Qm2Rt4Vw"
+AWS_ID = "AKIAI3F0DNN7EXA1B2C4"
+JWT = ("eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0."
+       "dOJvBm9vS2VyXzEyMzQ1Njc")
+B64_KEY = base64.b64encode(b"sk-S28B64Leak0001").decode()
+
+
+class TestVendorTokenPatterns(unittest.TestCase):
+    """Decisive vendor prefixes are redacted wherever they appear."""
+
+    def _assert_redacted(self, text, kind="api_key"):
+        out, hits = redact_text(text)
+        self.assertNotIn(SK_KEY, out)
+        self.assertIn("[REDACTED:", out)
+        self.assertTrue(hits)
+        self.assertEqual(hits[0][0], kind)
+        return out
+
+    def test_sk_key_in_prose(self):
+        self._assert_redacted(f"调用 API 时使用 {SK_KEY} 作为密钥。")
+
+    def test_sk_key_in_python_assignment(self):
+        out = self._assert_redacted(f'API_KEY = "{SK_KEY}"')
+        self.assertIn('API_KEY = "[REDACTED:api_key]"', out)
+
+    def test_sk_key_in_shell_export(self):
+        self._assert_redacted(f"export DASHSCOPE_API_KEY={SK_KEY}")
+
+    def test_github_pat(self):
+        out, hits = redact_text(f'GITHUB_TOKEN = "{GH_PAT}"')
+        self.assertNotIn(GH_PAT, out)
+        self.assertTrue(hits)
+
+    def test_aws_access_key_id(self):
+        out, _ = redact_text(f"aws_access_key_id = {AWS_ID}")
+        self.assertNotIn(AWS_ID, out)
+
+    def test_jwt(self):
+        out, hits = redact_text(f"token: {JWT}")
+        self.assertNotIn(JWT, out)
+        self.assertIn("[REDACTED:jwt]", out)
+        self.assertEqual(hits[0][0], "jwt")
+
+    def test_bearer_header_in_curl(self):
+        out, hits = redact_text(
+            f'curl -H "Authorization: Bearer {SK_KEY}" https://api.example.com')
+        self.assertNotIn(SK_KEY, out)
+        self.assertTrue(hits)
+
+    def test_sk_prefix_needs_a_left_boundary(self):
+        """``sk-`` inside an ordinary word is not a credential."""
+        for text in ("task-tracking pipeline", "ping",
+                     "risk-assessment model"):
+            out, hits = redact_text(text)
+            self.assertEqual(out, text, text)
+            self.assertEqual(hits, ())
+
+    def test_bearer_is_case_sensitive(self):
+        """Lowercase "bearer" is prose, not an Authorization header."""
+        text = ("HTTP headers are bearer credentials in disguise; "
+                "see the bearer endpoint docs.")
+        out, hits = redact_text(text)
+        self.assertEqual(out, text)
+        self.assertEqual(hits, ())
+
+
+class TestBase64Payload(unittest.TestCase):
+    """A persona instruction to decode a credential at runtime."""
+
+    def test_blob_decoding_to_a_key_is_redacted(self):
+        text = f'    echo "{B64_KEY}" | base64 -d'
+        out, hits = redact_text(text)
+        self.assertNotIn(B64_KEY, out)
+        self.assertIn("[REDACTED:base64]", out)
+        self.assertEqual(hits[0][0], "base64")
+
+    def test_plain_base64_of_benign_text_is_kept(self):
+        blob = base64.b64encode(b"hello world, this is fine").decode()
+        text = f"echo {blob} | base64 -d"
+        out, hits = redact_text(text)
+        self.assertEqual(out, text)
+        self.assertEqual(hits, ())
+
+    def test_hex_digest_is_not_a_payload(self):
+        """A sha256 is base64-alphabet-shaped but decodes to binary."""
+        digest = "d4735e3a265e16eee03f59718b9b5d03019c07d8b6c51f90da3a666eec13ab35"
+        text = f"commit {digest} is the release"
+        out, hits = redact_text(text)
+        self.assertEqual(out, text)
+        self.assertEqual(hits, ())
+
+    def test_data_uri_is_skipped(self):
+        payload = base64.b64encode(b"PNGDATA" * 12).decode()
+        text = f"![img](data:image/png;base64,{payload})"
+        out, hits = redact_text(text)
+        self.assertEqual(out, text)
+        self.assertEqual(hits, ())
+
+
+class TestDocumentedPlaceholdersSurvive(unittest.TestCase):
+    """The false-positive gate: docs are full of secret-shaped non-secrets.
+
+    Every case here is real content taken from this repo's docs, the framework
+    default templates and converted third-party agent packages.
+    """
+
+    CLEAN = (
+        # variable references / template placeholders
+        'Use `os.environ["DASHSCOPE_API_KEY"]` to read the key.',
+        "Pass `--api-key ` on the command line.",
+        "curl -H \"Authorization: Bearer $TOKEN\" https://api.example.com/v1",
+        'export API_KEY=""',
+        "api_key = ${DASHSCOPE_API_KEY}",
+        "headers = {'Authorization': f'Bearer {api_key}'}",
+        "url: https://gateway.example.com/v1?access_token=${token}",
+        "password = keyvaultref:$SECRET_URI",
+        "DB_PASSWORD = secretref:db-password",
+        # identifier-shaped values, not credentials
+        "apiKey = SendGridApiKey",
+        "api_key = MySecretValue123",
+        "page_token = options?.pageToken",
+        "credential = Invoke-MgGraphRequest",
+        "session_id = self.runtime.session_id",
+        "SECTION_KEY = personalization",
+        # documentation URLs and examples
+        "postgresql+asyncpg://user:pass@localhost/db",
+        "base_url: https://dashscope.aliyuncs.com/compatible-mode/v1",
+        "See https://example.com/docs?api_key=&model=qwen3-max",
+        # placeholder spellings
+        "OPENAI_API_KEY=sk-xxxxxxxxxxxxxxxxxxxx",
+        "api_key: sk-your-key-here",
+        'token = "changeme123456"',
+        # data fields that are NOT credentials
+        '{"usage": {"tokens": 1234, "keys": ["a"]}}',
+        "max_tokens: 4096",
+        "page_token: CaESBgoGEgQiBw",
+        '{"key": "user_profile_2"}',
+        # prose
+        "You are a helpful weather assistant. Query the forecast API.",
+    )
+
+    def test_clean_content_is_untouched(self):
+        for text in self.CLEAN:
+            with self.subTest(text=text):
+                out, hits = redact_text(text)
+                self.assertEqual(out, text)
+                self.assertEqual(hits, ())
+
+    def test_clean_multiline_document_round_trips(self):
+        doc = "\n".join(self.CLEAN)
+        out, hits = redact_text(doc)
+        self.assertEqual(out, doc)
+        self.assertEqual(hits, ())
+
+
+class TestNameStrength(unittest.TestCase):
+    """Which key names may drive a redaction, and how hard they push."""
+
+    def test_qualified_credential_names_are_strong(self):
+        for name in ("api_key", "apiKey", "model.api_key", "--auth-token",
+                     "GITHUB_TOKEN", "db_password", "smtp_passwd",
+                     "client_secret", "access_token", "OPENWEATHER_KEY"):
+            with self.subTest(name=name):
+                self.assertEqual(name_strength(name), 2)
+
+    def test_bare_singular_names_are_weak(self):
+        for name in ("key", "token", "secret", "password", "credential"):
+            with self.subTest(name=name):
+                self.assertEqual(name_strength(name), 1)
+
+    def test_data_and_metadata_names_never_trigger(self):
+        for name in ("max_tokens", "min_tokens", "total_tokens", "page_token",
+                     "next_token", "continuation_token", "session_id",
+                     "request_id", "trace_id", "tokens", "keys", "key_id",
+                     "key_name", "token_type", "secret_name", "options.pageToken",
+                     "id", "name", "description", "model", "base_url"):
+            with self.subTest(name=name):
+                self.assertEqual(name_strength(name), 0)
+
+
+class TestWeakNameValueGate(unittest.TestCase):
+    """A bare ``key``/``token`` only fires on a credential-shaped value."""
+
+    def test_memory_note_credential_is_redacted(self):
+        out, hits = redact_text("- gateway token: S42bMemTokenLeak01")
+        self.assertNotIn("S42bMemTokenLeak01", out)
+        self.assertEqual(hits[0][0], "credential")
+        self.assertEqual(hits[0][2], "token")
+
+    def test_documented_flag_value_is_redacted(self):
+        out, hits = redact_text(
+            "python leak_point.py --token S25dFlagValLeak01")
+        self.assertNotIn("S25dFlagValLeak01", out)
+        self.assertEqual(hits[0][0], "flag")
+
+    def test_data_mapping_value_survives(self):
+        for text in ('{"key": "user_profile_2"}', '{"token": "session_a1b2"}',
+                     "key: lowercase_snake_value_9"):
+            with self.subTest(text=text):
+                out, hits = redact_text(text)
+                self.assertEqual(out, text)
+                self.assertEqual(hits, ())
+
+
+class TestUrlCredentials(unittest.TestCase):
+    """URL credentials in free text, with the lenient value gate."""
+
+    def test_secret_query_parameter_is_blanked_name_kept(self):
+        out, hits = redact_text(
+            "url: https://api.example.com/v1?api_key=S34bDocUrlLeak01&m=1")
+        self.assertNotIn("S34bDocUrlLeak01", out)
+        self.assertIn("api_key=[REDACTED:api_key]", out)
+        self.assertIn("&m=1", out)
+        self.assertEqual(hits[0][2], "api_key")
+
+    def test_userinfo_password_is_redacted(self):
+        out, _ = redact_text("git clone https://user:Sup3rS3cretPass99@h/r")
+        self.assertNotIn("Sup3rS3cretPass99", out)
+        self.assertIn("user:[REDACTED:password]@h/r", out)
+
+    def test_bare_userinfo_token_is_redacted(self):
+        out, _ = redact_text("git clone https://ghp_AbCdEfG7h9IjK2LmNo@h/r")
+        self.assertNotIn("ghp_AbCdEfG7h9IjK2LmNo", out)
+
+    def test_documented_example_url_survives(self):
+        text = "postgresql+asyncpg://user:pass@localhost/db"
+        out, hits = redact_text(text)
+        self.assertEqual(out, text)
+        self.assertEqual(hits, ())
+
+    def test_variable_query_value_survives(self):
+        text = "https://gateway.example.com/v1?access_token=${token}"
+        out, hits = redact_text(text)
+        self.assertEqual(out, text)
+        self.assertEqual(hits, ())
+
+    def test_trailing_punctuation_is_not_swallowed(self):
+        out, _ = redact_text(
+            f"See https://h/v1?api_key=S34bDocUrlLeak01.")
+        self.assertTrue(out.endswith("."))
+        self.assertNotIn("S34bDocUrlLeak01", out)
+
+
+class TestTierAConfigShapedFiles(unittest.TestCase):
+    """Structural cleaning by name or shape, at ANY path."""
+
+    def _out(self, rel, text):
+        raw = text.encode("utf-8")
+        cleaned, hits = redact_outbound(rel, raw)
+        return cleaned.decode("utf-8"), hits
+
+    def test_skill_local_mcp_json_is_scrubbed(self):
+        payload = json.dumps({
+            "mcpServers": {
+                "weather": {
+                    "command": "uvx",
+                    "args": ["-y", "srv", "--api-key", "S32bArgValLeak001"],
+                    "env": {
+                        "OPENWEATHER_KEY": "S32EnvKeyLeak0001"
+                    },
+                    "url": "https://api.example.com/v1?api_key=S33UrlKeyLeak01",
+                }
+            }
+        })
+        out, hits = self._out("skills/weather/mcp.json", payload)
+        data = json.loads(out)
+        server = data["mcpServers"]["weather"]
+        self.assertEqual(server["env"], {"OPENWEATHER_KEY": ""})
+        self.assertIn("api_key=", server["url"])
+        self.assertNotIn("S33UrlKeyLeak01", server["url"])
+        self.assertNotIn("S32bArgValLeak001", out)
+        self.assertEqual(hits[0].kind, "config")
+
+    def test_mcp_shape_triggers_scrub_under_any_filename(self):
+        """The scrubber 'knows' this format, so the filename must not matter."""
+        payload = json.dumps({
+            "name": "weather",
+            "mcpServers": {
+                "w": {
+                    "env": {
+                        "KEY": "S34DocEnvLeak0001"
+                    }
+                }
+            },
+        })
+        out, hits = self._out("skills/weather/tools.json", payload)
+        self.assertNotIn("S34DocEnvLeak0001", out)
+        self.assertTrue(hits)
+
+    def test_skill_config_yaml_is_scrubbed(self):
+        text = ("name: weather\n"
+                "mcp_servers:\n"
+                "  weather:\n"
+                "    env:\n"
+                "      OPENWEATHER_KEY: S35YamlEnvLeak01\n"
+                "    url: https://api.example.com/v1?api_key=S35bYamlUrl01\n")
+        out, hits = self._out("skills/weather/config.yaml", text)
+        self.assertNotIn("S35YamlEnvLeak01", out)
+        self.assertNotIn("S35bYamlUrl01", out)
+        self.assertIn("OPENWEATHER_KEY: ''", out)
+        self.assertEqual(hits[0].kind, "config")
+
+    def test_skill_config_toml_is_scrubbed(self):
+        text = ('[mcp_servers.weather]\ncommand = "uvx"\n'
+                '[mcp_servers.weather.env]\nOPENWEATHER_KEY = "S36TomlEnv01"\n')
+        out, hits = self._out("skills/weather/config.toml", text)
+        self.assertNotIn("S36TomlEnv01", out)
+        self.assertTrue(hits)
+
+    def test_arbitrary_json_data_file_is_not_structurally_scrubbed(self):
+        """``tokens`` / ``keys`` are data fields; only the MCP subtree is
+        eligible for the bag rules."""
+        payload = json.dumps({
+            "usage": {
+                "tokens": 12345,
+                "keys": ["alpha", "beta"]
+            },
+            "key": "user_profile_2",
+        })
+        raw = payload.encode("utf-8")
+        cleaned, hits = redact_outbound("skills/x/data/usage.json", raw)
+        self.assertIs(cleaned, raw)
+        self.assertEqual(hits, ())
+
+    def test_mcp_subtree_scrub_leaves_the_rest_of_the_document(self):
+        payload = json.dumps({
+            "usage": {
+                "tokens": 12345
+            },
+            "mcpServers": {
+                "w": {
+                    "env": {
+                        "K": "S37MixedLeak00001"
+                    }
+                }
+            },
+        })
+        out, _ = self._out("skills/x/tools.json", payload)
+        data = json.loads(out)
+        self.assertEqual(data["usage"], {"tokens": 12345})
+        self.assertEqual(data["mcpServers"]["w"]["env"], {"K": ""})
+
+    def test_malformed_non_config_json_does_not_raise(self):
+        """A broken data fixture falls through to the text pass instead of
+        blocking the upload (the framework hook already fails closed for the
+        ROOT config files it owns)."""
+        raw = b'{"sources": [,,,'
+        cleaned, hits = redact_outbound("skills/x/broken.json", raw)
+        self.assertIs(cleaned, raw)
+        self.assertEqual(hits, ())
+
+    def test_secret_in_malformed_config_json_is_still_redacted(self):
+        raw = ('{"mcpServers": {"w": {"env": {"K": "%s"}},,,' % SK_KEY).encode()
+        cleaned, hits = redact_outbound("skills/x/broken.json", raw)
+        self.assertNotIn(SK_KEY, cleaned.decode("utf-8"))
+        self.assertTrue(hits)
+
+
+class TestRedactOutboundContract(unittest.TestCase):
+    """The invariants the sync paths rely on."""
+
+    def test_clean_content_returns_the_original_bytes_object(self):
+        """``drop_unchanged_defaults`` compares bytes and ``push_mirror`` skips
+        by sha256, so an untouched file must not be re-serialized."""
+        raw = b"# Persona\n\nYou are a helpful assistant.\n"
+        cleaned, hits = redact_outbound("SOUL.md", raw)
+        self.assertIs(cleaned, raw)
+        self.assertEqual(hits, ())
+
+    def test_non_utf8_binary_passes_through(self):
+        raw = b"\x89PNG\r\n\x1a\n" + bytes(range(256))
+        cleaned, hits = redact_outbound("skills/x/assets/logo.png", raw)
+        self.assertIs(cleaned, raw)
+        self.assertEqual(hits, ())
+
+    def test_redaction_is_idempotent(self):
+        text = (f"key = {SK_KEY}\n"
+                f"curl -H 'Authorization: Bearer {SK_KEY}'\n"
+                f"url: https://h/v1?api_key=S34bDocUrlLeak01\n"
+                f"export GITHUB_TOKEN={GH_PAT}\n")
+        once, hits1 = redact_text(text)
+        twice, hits2 = redact_text(once)
+        self.assertEqual(twice, once)
+        self.assertTrue(hits1)
+        self.assertEqual(hits2, ())
+
+    def test_outbound_redaction_is_idempotent(self):
+        raw = f"API_KEY = '{SK_KEY}'".encode("utf-8")
+        first, hits1 = redact_outbound("skills/x/run.py", raw)
+        second, hits2 = redact_outbound("skills/x/run.py", first)
+        self.assertEqual(second, first)
+        self.assertTrue(hits1)
+        self.assertEqual(hits2, ())
+
+    def test_findings_never_carry_the_secret(self):
+        raw = f"api_key = {SK_KEY}".encode("utf-8")
+        _cleaned, hits = redact_outbound("skills/x/env.sh", raw)
+        for finding in hits:
+            self.assertIsInstance(finding, Finding)
+            self.assertEqual(finding.rel, "skills/x/env.sh")
+            for field in finding:
+                self.assertNotIn(SK_KEY, str(field))
+
+    def test_finding_line_numbers_are_one_based(self):
+        text = "line one\nline two\napi_key = %s\n" % SK_KEY
+        _out, hits = redact_text(text)
+        self.assertEqual(hits[0][1], 3)
+
+    def test_jsonl_history_keeps_cursors_and_drops_keys(self):
+        """nanobot ``memory/history.jsonl`` mixes pagination cursors with
+        credentials; only the credential may go."""
+        line1 = json.dumps({"page_token": "CaESBgoGEgQiBw", "text": "hi"})
+        line2 = json.dumps({"api_key": SK_KEY, "text": "bye"})
+        raw = f"{line1}\n{line2}\n".encode("utf-8")
+        cleaned, hits = redact_outbound("memory/history.jsonl", raw)
+        text = cleaned.decode("utf-8")
+        self.assertIn("CaESBgoGEgQiBw", text)
+        self.assertNotIn(SK_KEY, text)
+        self.assertTrue(hits)
+
+    def test_never_raises_on_hostile_input(self):
+        """The watch daemon swallows exceptions and keeps polling, so a raise
+        here would silently stop all syncing."""
+        hostile = (
+            b"\xff\xfe\x00binary",
+            b"",
+            b"=" * 5000,
+            b"[]" * 2000,
+            b"sk-" * 500,
+            json.dumps({"a": ["b" * 300]}).encode(),
+            ("key = " + "A1" * 400).encode(),
+        )
+        for raw in hostile:
+            with self.subTest(raw=raw[:24]):
+                cleaned, _hits = redact_outbound("skills/x/any.md", raw)
+                self.assertIsInstance(cleaned, bytes)
+
+    def test_memo_returns_the_callers_object_for_clean_content(self):
+        """Two frameworks ship a byte-identical SOUL.md template, so the memo
+        is hit with equal content but a different bytes object."""
+        text = "# Persona\n\nNothing secret here.\n".encode("utf-8")
+        first, _ = redact_outbound("SOUL.md", text)
+        second, _ = redact_outbound("SOUL.md", bytes(text))
+        self.assertIs(second, text)
+        self.assertEqual(first, text)
+
+
+if __name__ == "__main__":
+    unittest.main()
diff --git a/tests/agent_hub/test_workspace.py b/tests/agent_hub/test_workspace.py
index a27cf6401..4635a573a 100644
--- a/tests/agent_hub/test_workspace.py
+++ b/tests/agent_hub/test_workspace.py
@@ -1,5 +1,6 @@
 # Copyright (c) Alibaba, Inc. and its affiliates.
 """Sub-agent-aware workspace spec collection tests."""
+import base64
 import json
 import tempfile
 import unittest
@@ -1605,5 +1606,186 @@ def test_toml_single_quoted_url(self):
         self.assertEqual(out, "base_url = 'https://h.example.com/v1?token='\n")
 
 
+class TestOutboundCoverageAcrossFrameworks(unittest.TestCase):
+    """BUG-0909-01: outbound cleaning must be content-driven, not path-driven.
+
+    Every framework collects ``skills/*`` recursively plus its persona and
+    memory documents, but the per-framework ``sanitize_outbound_file`` hooks
+    select files by PATH: ms-agent whitelisted ``settings.json`` / ``mcp.json``,
+    qwenpaw only ``agent.json``, hermes / openhuman only their root config, and
+    openclaw / nanobot / qoder defined no hook at all. A key an AI assistant
+    wrote into a skill script, a skill-local ``mcp.json``, ``SOUL.md`` or a
+    memory file was therefore uploaded verbatim into the remote repo and its
+    git history.
+    """
+
+    B64_KEY = base64.b64encode(b"sk-PersonaB64Leak001").decode()
+    SENTINELS = (
+        "sk-PersonaProseLeak01",
+        "sk-PersonaBearerLeak1",
+        B64_KEY,
+        "sk-SkillScriptLeak01",
+        "ghp_SkillPat7x9Qm2Rt4V",
+        "S32SkillEnvLeak0001",
+        "sk-MemoryNoteLeak0001",
+    )
+    BENIGN = (
+        "You are a helpful weather assistant.",
+        'os.environ.get("OPENWEATHER_KEY", "")',
+        "--api-key ",
+        "Bearer $OPENAI_API_KEY",
+        "Query the forecast.",
+    )
+
+    # The persona slot every framework collects.
+    PERSONA = {
+        "ms-agent": "SOUL.md",
+        "qwenpaw": "SOUL.md",
+        "hermes": "SOUL.md",
+        "openclaw": "SOUL.md",
+        "openhuman": "SOUL.md",
+        "nanobot": "SOUL.md",
+        "qoder": "AGENTS.md",
+    }
+    # The memory slot, where the framework has one (ms-agent keeps memory
+    # project-level, so its global home carries none).
+    MEMORY = {
+        "qwenpaw": "MEMORY.md",
+        "openclaw": "MEMORY.md",
+        "openhuman": "MEMORY.md",
+        "nanobot": "memory/MEMORY.md",
+        "qoder": "memory/MEMORY.md",
+        "hermes": "memories/MEMORY.md",
+    }
+    SCRIPT = "skills/weather/scripts/leak_point.py"
+    SKILL_MCP = "skills/weather/mcp.json"
+    SKILL_DOC = "skills/weather/SKILL.md"
+
+    def setUp(self):
+        self.tmp = tempfile.TemporaryDirectory()
+
+    def tearDown(self):
+        self.tmp.cleanup()
+
+    def _files(self, framework):
+        persona = (
+            "# Persona\n\n"
+            "You are a helpful weather assistant.\n\n"
+            "调用 API 时使用 sk-PersonaProseLeak01 作为密钥。\n\n"
+            "```bash\n"
+            'curl -H "Authorization: Bearer sk-PersonaBearerLeak1" https://h/v1\n'
+            "```\n\n"
+            f'    echo "{self.B64_KEY}" | base64 -d\n\n'
+            "Benign lines that must survive:\n\n"
+            "- Pass `--api-key ` on the command line.\n")
+        script = (
+            '"""Fetch the forecast."""\n'
+            "import os\n\n"
+            'API_KEY = "sk-SkillScriptLeak01"\n'
+            'GITHUB_TOKEN = "ghp_SkillPat7x9Qm2Rt4V"\n\n\n'
+            "def fetch(city):\n"
+            '    return city, os.environ.get("OPENWEATHER_KEY", "")\n')
+        skill_mcp = json.dumps({
+            "mcpServers": {
+                "weather": {
+                    "command": "uvx",
+                    "env": {"OPENWEATHER_KEY": "S32SkillEnvLeak0001"},
+                }
+            }
+        })
+        memory = ("# Memory\n\n## 凭据备忘\n\n"
+                  "- OPENAI_API_KEY=sk-MemoryNoteLeak0001\n")
+        # No bundled frontmatter markers: hermes / qwenpaw only keep a skill
+        # directory whose SKILL.md is user-authored.
+        skill_doc = (
+            "---\nname: weather\ndescription: Query the forecast.\n---\n\n"
+            "# Weather\n\n"
+            "```bash\n"
+            'curl -H "Authorization: Bearer $OPENAI_API_KEY" https://h/v1\n'
+            "```\n")
+        files = {
+            self.PERSONA[framework]: persona,
+            self.SCRIPT: script,
+            self.SKILL_MCP: skill_mcp,
+            self.SKILL_DOC: skill_doc,
+        }
+        memory_rel = self.MEMORY.get(framework)
+        if memory_rel:
+            files[memory_rel] = memory
+        return files
+
+    def _outbound(self, framework):
+        """Seed *framework*'s own layout and run the real upload sanitize path."""
+        from ms_agent.agent_hub._sync import sanitize_outbound
+
+        root = Path(self.tmp.name) / framework
+        root.mkdir(parents=True, exist_ok=True)
+        ws = build_spec(framework, "default", str(root)).workspace_root
+        files = self._files(framework)
+        for rel, content in files.items():
+            target = ws / rel
+            target.parent.mkdir(parents=True, exist_ok=True)
+            target.write_text(content, encoding="utf-8")
+        # Rebuild after seeding: a framework may resolve its data root from the
+        # markers it finds (openhuman probes for a live workspace).
+        spec = build_spec(framework, "default", str(root))
+        collected = spec.collect_bytes()
+        findings = []
+        return files, collected, sanitize_outbound(collected, spec,
+                                                   findings=findings), findings
+
+    def test_every_framework_redacts_skills_persona_and_memory(self):
+        for framework in sorted(FRAMEWORK_REGISTRY):
+            with self.subTest(framework=framework):
+                files, collected, out, findings = self._outbound(framework)
+                # Guard against a vacuous pass: the carriers really were
+                # collected by this framework's patterns.
+                self.assertIn(self.PERSONA[framework], collected)
+                self.assertIn(self.SCRIPT, collected)
+                blob = "\n".join(v.decode("utf-8", "replace")
+                                 for v in out.values())
+                for sentinel in self.SENTINELS:
+                    if sentinel == "sk-MemoryNoteLeak0001" \
+                            and framework not in self.MEMORY:
+                        continue
+                    self.assertNotIn(sentinel, blob)
+                self.assertTrue(findings, "no redaction reported")
+                for finding in findings:
+                    for field in finding:
+                        for sentinel in self.SENTINELS:
+                            self.assertNotIn(sentinel, str(field))
+
+    def test_benign_documentation_survives_in_every_framework(self):
+        for framework in sorted(FRAMEWORK_REGISTRY):
+            with self.subTest(framework=framework):
+                _files, _collected, out, _findings = self._outbound(framework)
+                blob = "\n".join(v.decode("utf-8", "replace")
+                                 for v in out.values())
+                for needle in self.BENIGN:
+                    self.assertIn(needle, blob)
+
+    def test_clean_workspace_is_not_rewritten(self):
+        """Byte identity matters: ``drop_unchanged_defaults`` compares bytes and
+        ``push_mirror`` skips uploads by sha256."""
+        for framework in sorted(FRAMEWORK_REGISTRY):
+            with self.subTest(framework=framework):
+                root = Path(self.tmp.name) / f"clean-{framework}"
+                root.mkdir(parents=True, exist_ok=True)
+                ws = build_spec(framework, "default", str(root)).workspace_root
+                rel = self.PERSONA[framework]
+                (ws / rel).parent.mkdir(parents=True, exist_ok=True)
+                (ws / rel).write_text(
+                    "# Persona\n\nYou are a helpful weather assistant.\n",
+                    encoding="utf-8")
+                spec = build_spec(framework, "default", str(root))
+                collected = spec.collect_bytes()
+                self.assertIn(rel, collected)
+                findings = []
+                from ms_agent.agent_hub._sync import sanitize_outbound
+                out = sanitize_outbound(collected, spec, findings=findings)
+                self.assertEqual(out[rel], collected[rel])
+                self.assertEqual(findings, [])
+
+
 if __name__ == "__main__":
     unittest.main()

From 5393018c413545d62d92f41903260d5b7170a8ec Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E6=9D=A8=E5=A0=83?= 
Date: Thu, 10 Sep 2026 17:14:04 +0800
Subject: [PATCH 3/3] refactor(agent_hub): trim outbound redaction comments and
 tests

The comments added with the content-driven redaction layer restated the code,
repeated the same rationale twice in one file, and carried a bug narrative that
belongs in the commit history. What remains is only the non-obvious why: the
case-sensitive Bearer match, the sk- left boundary, why scrub_url_secrets is not
reused for free text, why the base64 pass is not behind the prefilter, and the
never-raise / original-bytes / idempotent contracts. Also fixes the vendor
labels, which were shifted one line off their patterns.

Test methods are consolidated, not thinned: the vendor cases now cover all
eleven prefix branches instead of two, and the homogeneous false-positive guards
became subTest tables. Four redundant cases left the repo suite -- a multiline
rerun of an already per-line corpus, a trivial report-formatting assertion, a
per-framework benign check now folded into the leak test, and a dry-run report
check already covered by two existing tests.

No behavior change: the leak repro still reports 0 and the false-positive
benchmark still reports 0 hits over 690 clean files. Suite goes 369 -> 361
passed with subtests 93 -> 102.
---
 ms_agent/agent_hub/_commands.py            |   3 +-
 ms_agent/agent_hub/_secrets.py             | 260 ++++++++-------------
 ms_agent/agent_hub/_sync.py                |  38 ++-
 ms_agent/agent_hub/_watcher.py             |   1 -
 ms_agent/agent_hub/_workspace.py           |  16 +-
 ms_agent/agent_hub/frameworks/openhuman.py |  10 +-
 tests/agent_hub/test_cli.py                |  25 +-
 tests/agent_hub/test_secrets.py            |  99 ++++----
 tests/agent_hub/test_workspace.py          |  31 +--
 9 files changed, 176 insertions(+), 307 deletions(-)

diff --git a/ms_agent/agent_hub/_commands.py b/ms_agent/agent_hub/_commands.py
index 30c093953..3ef4bf2a9 100644
--- a/ms_agent/agent_hub/_commands.py
+++ b/ms_agent/agent_hub/_commands.py
@@ -453,8 +453,7 @@ def cmd_upload(
         color=display.COLOR_WRITTEN,
     )
     if redacted:
-        # Reported before the dry-run return so `--dry-run` shows exactly what
-        # a real upload would strip. A Finding never carries the secret text.
+        # Before the dry-run return, so --dry-run shows what would be stripped.
         display.table(
             'Secrets redacted',
             [(f.rel, f.kind, f'line {f.line}' if f.line else 'structural')
diff --git a/ms_agent/agent_hub/_secrets.py b/ms_agent/agent_hub/_secrets.py
index fe7f7c266..abca6d1c3 100644
--- a/ms_agent/agent_hub/_secrets.py
+++ b/ms_agent/agent_hub/_secrets.py
@@ -1,55 +1,28 @@
 # Copyright (c) ModelScope Contributors. All rights reserved.
 """Content-driven outbound secret redaction (BUG-0909-01).
 
-Why this module exists
-----------------------
-:meth:`WorkspaceSpec.sanitize_outbound_file` decides *whether* to clean a file
-by its PATH: each framework whitelists its own root config (ms-agent
-``settings.json`` / ``mcp.json``, qwenpaw ``agent.json``, hermes
-``config.yaml``, openhuman ``config.toml``) and returns every other collected
-file verbatim -- openclaw, nanobot and qoder define no hook at all. The
-collect patterns, however, take ``skills/*`` recursively plus the persona and
-memory documents, so a key that an AI assistant wrote into a skill script, a
-skill-local ``mcp.json``, ``SOUL.md`` or ``MEMORY.md`` was uploaded verbatim
-into the remote repo and its git history.
-
-This layer decides on CONTENT instead of path. It runs *after* the
-per-framework hook (see :func:`ms_agent.agent_hub._sync.sanitize_outbound`),
-so the structural cleaning and the fail-closed refusals for the known config
-files keep their behavior, and every framework -- present or future -- is
-covered without editing a single spec.
-
-Two tiers
----------
-* **Tier A (config-shaped)** -- a file that IS configuration, by name
-  (``mcp.json``, ``config.yaml``, ...) or by shape (its parsed JSON/YAML/TOML
-  carries an ``mcpServers`` / ``mcp_servers`` mapping), is cleaned with the
-  structural scrubbers in :mod:`._workspace`. A shape-triggered file is
-  cleaned only inside that MCP subtree: the shared vocabulary blanks any key
-  named ``tokens`` / ``keys`` / ``session_id``, which would destroy legitimate
-  data in a skill's JSON fixture or a memory dump.
-* **Tier B (everything else)** -- documents, scripts, notebooks, JSONL: only
-  high-confidence secret *values* are replaced with a ``[REDACTED:]``
-  marker. The bag rules (``env`` / ``headers`` cleared wholesale) never apply
-  here, and the name vocabulary is narrower than :func:`is_secret_key`
-  (``max_tokens`` / ``page_token`` / ``session_id`` are pagination and
-  telemetry fields, not credentials).
-
-Contract
---------
-* **Never raises.** The watch daemon swallows exceptions and keeps polling
-  (``_watcher._poll_once``), so a raise here would silently stop all syncing.
-* **Returns the original bytes object when nothing was redacted.**
-  Re-serializing a parsed config reformats it, which breaks both
-  :func:`._sync.drop_unchanged_defaults` (byte comparison against the
-  framework default templates) and the sha256 idempotent-skip in
-  ``push_mirror`` / ``push_resources``.
-* **Deterministic and idempotent.** The watcher stores the sha256 of the
-  SANITIZED bytes as its sync baseline, so an unstable rewrite would re-push
-  every cycle; a ``[REDACTED:...]`` marker never re-matches any rule.
-* **Outbound only.** ``sanitize_inbound_file`` also serves local ``convert``
-  writes, where redaction would strip the user's own keys from the converted
-  agent and leave it unable to run.
+``WorkspaceSpec.sanitize_outbound_file`` picks files by PATH -- each framework
+whitelists its own root config and returns everything else verbatim, and
+openclaw / nanobot / qoder define no hook at all -- while the collect patterns
+take ``skills/*`` plus the persona and memory documents. This layer picks by
+CONTENT instead and runs *after* that hook (see
+:func:`._sync.sanitize_outbound`), so no framework spec needs editing.
+
+* **Tier A** -- config-shaped files, by name (``mcp.json``) or by an
+  ``mcpServers`` shape trigger, get the structural scrubbers from
+  :mod:`._workspace`. A shape-triggered file is cleaned inside that subtree
+  only: the shared vocabulary blanks any key named ``tokens`` / ``keys``, which
+  would destroy data in a skill's JSON fixture or a memory dump.
+* **Tier B** -- every other text file gets high-confidence secret VALUES
+  replaced with ``[REDACTED:]``, under a narrower name vocabulary and a
+  value gate tuned to leave documentation alone.
+
+Contract: never raises (the watch daemon swallows exceptions, so a raise would
+silently stop all syncing); returns the ORIGINAL bytes object when nothing was
+redacted (``drop_unchanged_defaults`` and the sha256 push-skip both compare
+bytes); deterministic and idempotent (the watcher baselines the sanitized sha);
+outbound only (``sanitize_inbound_file`` also serves local ``convert`` writes,
+where redaction would strip the user's own keys from the converted agent).
 """
 from __future__ import annotations
 
@@ -72,12 +45,10 @@
 
 
 class Finding(NamedTuple):
-    """One redacted secret.
+    """One redacted secret: kind and key name, never the secret text itself.
 
-    Carries the *kind* and the key name, never the secret text itself, so the
-    upload report and the watch log cannot leak what they are reporting.
-    ``line`` is 1-based; Tier A (structural config cleaning) reports line 0
-    because a structural scrub has no single location.
+    ``line`` is 1-based, or 0 for a Tier A structural scrub, which has no
+    single location.
     """
     rel: str
     kind: str
@@ -90,15 +61,14 @@ class Finding(NamedTuple):
 # ---------------------------------------------------------------------------
 
 # Vendor-prefixed credentials. The prefix is decisive on its own, so only the
-# placeholder / variable-reference gate applies to these matches -- a real key
-# may legitimately have no digit or an unusual charset. The lookbehind keeps
-# ``sk-`` from firing inside ordinary words (``task-tracking``,
-# ````).
+# placeholder gate applies to these matches -- a real key may have no digit or
+# an unusual charset. The lookbehind keeps ``sk-`` from firing inside ordinary
+# words (``task-tracking``, ````).
 _VENDOR_TOKEN_RE = re.compile(
     r'(?["\']?)'
                         r'(?P[^\s"\'`,;)\]}>]+)')
 
-# ``--flag VALUE`` / ``--flag=VALUE`` / ``-f VALUE`` on a documented command
-# line.
 _FLAG_RE = re.compile(
     r'(?[A-Za-z][A-Za-z0-9_.\-]{1,62})'
     r'(?:[= \t]+(?P["\']?)'
@@ -147,20 +115,17 @@ class Finding(NamedTuple):
                         r'(?![A-Za-z0-9+/=])')
 _BASE64_MAX_ATTEMPTS = 400
 
-# Values that are documentation placeholders rather than credentials. Only
-# unambiguous markers qualify: a 5+ character word, a repeated-character run
-# (``xxxx``), or filler digits. Short words (``foo``, ``test``, ``none``) are
-# deliberately absent -- they occur inside a random base62 credential often
-# enough (~1 in 1000) that treating them as placeholders would let real keys
-# through, and a missed key is worse than a redacted doc example.
+# Documentation placeholders rather than credentials. Short common words
+# (``foo``, ``test``, ``none``) are deliberately absent: they occur inside a
+# random base62 credential often enough (~1 in 1000) that treating them as
+# placeholders would let real keys through.
 _PLACEHOLDER_RE = re.compile(
     r'(?:your|yours|xxx+|yyy+|zzz+|example|dummy|fake|sample|placeholder|'
     r'changeme|change_me|redacted|todo|fixme|localhost|something|whatever|'
     r'unknown|mysecret|secretvalue|000000|123456)', re.IGNORECASE)
 
-# Characters that mark a value as an expression / path / reference rather than
-# a credential (``keyvaultref:$SECRET_URI``, ``options?.pageToken``,
-# ``${API_KEY}``, ``/etc/keys/a.pem``).
+# Characters marking a value as an expression, path or reference rather than a
+# credential (``keyvaultref:$SECRET_URI``, ``${API_KEY}``, ``/etc/keys/a.pem``).
 _BAD_VALUE_CHARS = frozenset('$={}?*()@/\\`|&<>:,#%')
 
 # A fully base64-ish value is allowed to carry ``/``, ``+`` and ``=`` padding.
@@ -168,16 +133,11 @@ class Finding(NamedTuple):
 
 _CAMEL_RE = re.compile(r'[a-z][A-Z]')
 
-# Tier B name vocabulary, in three strengths. Deliberately NARROWER than
-# :func:`._workspace.is_secret_key`: bare plural ``tokens`` / ``keys`` and the
-# pagination / telemetry spellings in :data:`_NAME_DENY` are ordinary data
-# fields (a nanobot ``memory/history.jsonl`` cursor, an LLM ``max_tokens``
-# setting) whose values are high-entropy digit-bearing strings -- exactly what
-# a credential looks like. ``sk`` is dropped too: it is meaningless as a key
-# name and real ``sk-`` values are caught by :data:`_VENDOR_TOKEN_RE`.
-#
-# STRONG names (``api_key``, ``access_token``, ``client_secret``, ...) are
-# unambiguous: a credential is what they hold.
+# Tier B name vocabulary, in three strengths, deliberately NARROWER than
+# :func:`._workspace.is_secret_key`: the spellings in :data:`_NAME_DENY` are
+# data fields (an LLM ``max_tokens``, a ``memory/history.jsonl`` cursor) whose
+# values are high-entropy digit-bearing strings -- exactly what a credential
+# looks like. STRONG names are unambiguous credential holders.
 _STRONG_NAME_RE = re.compile(
     r'(?:^|[_\-.])(?:'
     r'api[_-]?keys?|apikeys?|access[_-]?keys?|secret[_-]?keys?|'
@@ -187,18 +147,14 @@ class Finding(NamedTuple):
     r'auth[_-]?key|secret[_-]?key|master[_-]?key|signing[_-]?key)$',
     re.IGNORECASE)
 
-# WEAK names are BARE singulars. They do hold credentials in a memory note
-# ("gateway token: ...") or a documented command line (``--token ...``), but
-# they are also ordinary data-mapping fields (``{"key": "user_profile_2"}``),
-# so they only fire against a stricter value gate. The same word WITH a
-# qualifier (``github_token``, ``db_password``, ``smtp_passwd``) is
-# unambiguous and counts as strong.
+# WEAK names are BARE singulars: they hold credentials in a memory note
+# ("gateway token: ...") but are also ordinary data fields
+# (``{"key": "user_profile_2"}``), so they face a stricter value gate. The same
+# word with a qualifier (``github_token``, ``db_password``) counts as strong.
 _WEAK_NAME_RE = re.compile(
     r'(?:^|[_\-.])(?:keys?|tokens?|secrets?|passwords?|passwd|'
     r'credentials?|cookies?|bearer)$', re.IGNORECASE)
 
-# Bare stems, singularized: a name whose normalized form IS one of these (and
-# nothing more) is weak.
 _WEAK_BARE_STEMS = frozenset((
     'key',
     'token',
@@ -210,13 +166,8 @@ class Finding(NamedTuple):
     'bearer',
 ))
 
-# Names that are never a trigger. Plurals are data fields (an LLM
-# ``max_tokens`` setting, a nanobot ``memory/history.jsonl`` cursor) whose
-# values are high-entropy digit-bearing strings -- exactly what a credential
-# looks like -- and the metadata spellings describe a secret without being
-# one. ``sk`` is absent from every vocabulary for the same reason: it is
-# meaningless as a key name, and real ``sk-`` values are caught by
-# :data:`_VENDOR_TOKEN_RE`.
+# Names that are never a trigger. The metadata spellings describe a secret
+# without being one.
 _NAME_DENY = frozenset((
     # pagination / telemetry cursors
     'page_token',
@@ -248,22 +199,16 @@ class Finding(NamedTuple):
     'secret_id',
 ))
 
-# Name strength: 0 = never a trigger, 1 = weak (strict value gate),
-# 2 = strong.
 _DENY, WEAK, STRONG = 0, 1, 2
 
 
 def name_strength(name: str) -> int:
-    """How much a key / flag *name* is allowed to drive Tier B redaction.
+    """How much a key / flag *name* may drive Tier B redaction.
 
     ``name`` may be dotted or dashed (``model.api_key``, ``--auth-token``).
-    An explicit strong spelling wins first (``client_secrets`` and
-    ``access_tokens`` are credentials even though their last segment is a
-    denied plural), then the deny list, then the generic vocabulary -- where a
-    BARE singular is weak and a qualified one (``github_token``,
-    ``db_password``) is strong. Testing the last segment is what keeps
-    ``options.pageToken`` from being a trigger at all: there is no separator
-    before ``token``.
+    Strong spellings are tested BEFORE the deny list, because
+    ``client_secrets`` ends in a denied plural yet is a credential. Testing the
+    last segment is what keeps ``options.pageToken`` from triggering at all.
     """
     full = name.strip().strip('\'"').lstrip('-').lower()
     if not full:
@@ -289,31 +234,23 @@ def _shannon_entropy(value: str) -> float:
         (c / total) * math.log2(c / total) for c in Counter(value).values())
 
 
-# Lowercase snake_case / kebab-case is how a data-mapping value or a doc
-# placeholder spells itself (``user_profile_2``, ``my-super-secret-1``); real
-# credentials are hex, base64 or mixed-case. Only applied to WEAK names, so a
-# strongly named ``api_key`` keeps its value whatever the casing.
+# Lowercase snake_case / kebab-case is how a data value or a doc placeholder
+# spells itself; only applied to WEAK names.
 _SNAKE_CASE_RE = re.compile(r'^[a-z0-9]+(?:[_\-][a-z0-9]+)+$')
 
-# Letter-words with at most TRAILING digits: the shape of a variable or field
-# name (``SendGridApiKey``, ``MySecretValue123``, ``options.pageToken``), not
-# of a credential. A real key interleaves digits with letters
-# (``S42bMemTokenLeak01``), which fails this shape and stays eligible.
+# Letter-words with at most TRAILING digits are variable names
+# (``SendGridApiKey``, ``MySecretValue123``); a real key interleaves digits
+# with letters (``S42bMemTokenLeak01``) and stays eligible.
 _IDENTIFIER_SHAPE_RE = re.compile(r'^[A-Za-z][A-Za-z_]*[0-9]*$')
 
 
 def _looks_real(value: str, quoted: bool, strength: int = STRONG) -> bool:
     """Whether *value* looks like a real credential rather than a placeholder.
 
-    This gate is what keeps the redactor from eating documentation. A value
-    must be long enough, carry at least one digit, be high-entropy, and must
-    not be a variable reference, a placeholder word, an expression, a path, or
-    a camelCase identifier (``SendGridApiKey``, ``options.pageToken``). Fully
-    base64-shaped values are exempt from the punctuation ban and the identifier
-    test, since real keys legitimately carry ``/``, ``+`` and ``=`` padding.
-
-    A WEAK name (bare ``key`` / ``token`` / ``secret`` / ...) raises every
-    threshold and additionally refuses lowercase snake_case values.
+    This gate is what keeps the redactor from eating documentation. Fully
+    base64-shaped values skip the punctuation ban and the identifier test,
+    since real keys carry ``/``, ``+`` and ``=`` padding. A WEAK name raises
+    every threshold and also refuses lowercase snake_case values.
     """
     val = value.strip().strip('\'"`')
     if not val:
@@ -390,12 +327,10 @@ def repl(m):
 def _redact_urls(text: str, hits: list) -> str:
     """Strip credentials from URLs embedded in free text.
 
-    Lenient sibling of :func:`._workspace.scrub_url_secrets`, which is written
+    Not a reuse of :func:`._workspace.scrub_url_secrets`: that one is written
     for config scalars and blanks a secret-named query parameter even when its
     value is a variable reference (``?access_token=${token}`` in a converted
-    ``AGENTS.md``). Here every credential candidate passes :func:`_looks_real`
-    first, so documentation examples such as
-    ``postgresql+asyncpg://user:pass@host`` survive.
+    ``AGENTS.md``). Here every candidate passes :func:`_looks_real` first.
     """
 
     def repl(m):
@@ -427,7 +362,7 @@ def repl(m):
                     userinfo, quoted=False):
                 # Colon-less userinfo is how PAT-style tokens travel
                 # (``https://ghp_xxx@host``) and cannot be told from a
-                # username: fail closed, as ``scrub_url_secrets`` does.
+                # username, so fail closed.
                 authority = f'{_marker("token")}@' + authority[at + 1:]
                 changed = True
                 hits.append(('token', _line_of(text, m.start()), ''))
@@ -497,10 +432,10 @@ def repl(m):
 def _redact_base64(text: str, hits: list) -> str:
     """Replace a base64 payload whose DECODED content is a secret.
 
-    Persona files sometimes carry "decode this at runtime" instructions whose
-    plaintext never appears in the file. Only blobs that decode to valid
-    printable text matching a credential rule are touched, so hashes (invalid
-    UTF-8), data URIs and ordinary identifiers survive.
+    Persona files carry "decode this at runtime" instructions whose plaintext
+    never appears in the file. Only blobs decoding to printable text that
+    matches a credential rule are touched, so hashes, data URIs and ordinary
+    identifiers survive.
     """
     attempts = 0
 
@@ -538,12 +473,10 @@ def repl(m):
     return _BASE64_RE.sub(repl, text)
 
 
-# One cheap union of every Tier B textual trigger. A file that matches none of
-# these cannot be redacted by any of the gated passes, so skipping them is
-# lossless -- and the watch daemon re-runs this over the whole workspace every
-# poll. Deliberately over-matches (``monkey`` contains ``key``): it is a gate,
-# not a rule. The base64 pass is NOT gated, because an encoded payload carries
-# no textual signal at all.
+# Cheap union of every Tier B textual trigger: a file matching none of these
+# cannot be redacted by the gated passes, so skipping them is lossless. It
+# deliberately over-matches (``monkey`` contains ``key``) -- a gate, not a rule.
+# The base64 pass is NOT gated: an encoded payload carries no textual signal.
 _PREFILTER_RE = re.compile(
     r'(?:sk-|gh[pousr]_|github_pat_|glpat-|xox[baprs]-|AKIA|AIza|hf_|npm_|'
     r'shpat_|eyJ|[Bb]earer|://|'
@@ -554,12 +487,10 @@ def repl(m):
 def redact_text(text: str) -> tuple[str, tuple[tuple[str, int, str], ...]]:
     """Redact high-confidence secret values in free text (Tier B).
 
-    Returns ``(text, hits)`` where each hit is ``(kind, line, name)``. The
-    input is returned unchanged (the same ``str`` object) when nothing
-    matched. Passes run in a fixed order -- base64, vendor prefixes, JWT,
-    ``Bearer``, URLs, ``NAME=VALUE``, ``--flag VALUE`` -- so a secret is
-    replaced by the most specific rule that sees it first and the later passes
-    only ever encounter a ``[REDACTED:...]`` marker, which no rule matches.
+    Returns ``(text, hits)`` with each hit ``(kind, line, name)``, and the
+    input unchanged when nothing matched. The pass order is fixed so the most
+    specific rule sees a secret first; later passes only ever encounter a
+    ``[REDACTED:...]`` marker, which no rule matches.
     """
     if not text:
         return text, ()
@@ -610,13 +541,10 @@ def _json_has_mcp(obj) -> bool:
 
 
 def _scrub_json_mcp_subtrees(obj) -> bool:
-    """Clean only the ``mcpServers`` subtrees of a parsed JSON document.
+    """Apply the full structural policy inside ``mcpServers`` subtrees only.
 
-    A skill's ``mcp.json``-shaped payload embedded in an otherwise unrelated
-    JSON document gets the full structural policy (``env`` / ``headers`` bags
-    cleared, ``args`` command lines scrubbed, URL credentials stripped) while
-    the rest of the document -- which may legitimately hold ``tokens`` or
-    ``keys`` data fields -- is left alone. Returns whether anything changed.
+    The rest of the document may legitimately hold ``tokens`` / ``keys`` data
+    fields. Returns whether anything changed.
     """
     changed = False
     if isinstance(obj, dict):
@@ -640,11 +568,10 @@ def _redact_config(rel_path: str, text: str, hits: list[tuple[str, int,
                                                               str]]) -> str:
     """Tier A: structural cleaning of a config-shaped file at any path.
 
-    Best effort by design. A known config name that fails to parse falls
-    through to Tier B instead of raising: refusing to upload a skill's JSON
-    data fixture would be worse than redacting the secret values inside it.
-    (The per-framework hook already fails closed for the ROOT config files it
-    owns, and it runs before this layer.)
+    Best effort: a config that fails to parse falls through to Tier B instead
+    of raising, because refusing to upload a skill's JSON data fixture is worse
+    than redacting the secrets inside it. The per-framework hook already fails
+    closed for the ROOT config files it owns, and runs first.
     """
     base = rel_path.rsplit('/', 1)[-1].lower()
     if base.endswith('.json') or base.endswith('.jsonl'):
@@ -688,10 +615,9 @@ def _redact_config(rel_path: str, text: str, hits: list[tuple[str, int,
 # entry point
 # ---------------------------------------------------------------------------
 
-# Bounded sha256 -> result memo. The watcher re-sanitizes the whole workspace
-# every poll and upload re-runs it per invocation, so identical content is
-# scanned once. Capped on both entry count and per-file size to keep the
-# daemon's footprint flat.
+# Bounded sha256 -> result memo: the watcher re-sanitizes the whole workspace
+# every poll, so identical content is scanned once. Capped on entry count and
+# per-file size to keep the daemon's footprint flat.
 _MEMO_MAX_ENTRIES = 128
 _MEMO_MAX_FILE_SIZE = 128 * 1024
 _memo: 'OrderedDict[tuple[str, str], tuple[bytes, tuple[Finding, ...]]]' = \
@@ -702,7 +628,6 @@ def _redact(rel_path: str, raw: bytes) -> tuple[bytes, tuple[Finding, ...]]:
     try:
         text = raw.decode('utf-8')
     except UnicodeDecodeError:
-        # Binary asset (image, PDF, archive): not a text-secret carrier.
         return raw, ()
 
     hits: list[tuple[str, int, str]] = []
@@ -720,11 +645,9 @@ def redact_outbound(rel_path: str,
                     raw: bytes) -> tuple[bytes, tuple[Finding, ...]]:
     """Redact secrets in one collected file, by content rather than by path.
 
-    Runs after the framework's own :meth:`sanitize_outbound_file` hook. Never
-    raises: an unparseable, oversized or exotic file is returned unchanged
-    (Tier B still gets a best-effort pass at its text), because a crashed
-    sanitize would either block the upload or -- inside the watch daemon,
-    which swallows exceptions -- silently stop syncing.
+    Runs after the framework's own :meth:`sanitize_outbound_file` hook and
+    never raises: a crashed sanitize would block the upload or, inside the
+    watch daemon, silently stop syncing.
     """
     try:
         digest = hashlib.sha256(raw).hexdigest()
@@ -733,9 +656,8 @@ def redact_outbound(rel_path: str,
         if cached is not None:
             _memo.move_to_end(memo_key)
             cleaned, hits = cached
-            # The cached bytes came from an earlier call, so hand back THIS
-            # caller's object when nothing was redacted: the contract is to
-            # return the original, not merely an equal copy.
+            # The cached bytes belong to an earlier caller: hand back THIS one's
+            # object when nothing was redacted.
             return (raw if cleaned == raw else cleaned), hits
         result = _redact(rel_path, raw)
         if len(raw) <= _MEMO_MAX_FILE_SIZE:
diff --git a/ms_agent/agent_hub/_sync.py b/ms_agent/agent_hub/_sync.py
index edfb5d3ad..33aa8b140 100644
--- a/ms_agent/agent_hub/_sync.py
+++ b/ms_agent/agent_hub/_sync.py
@@ -81,30 +81,20 @@ def sanitize_outbound(resources: dict,
                       findings: list | None = None) -> dict:
     """Strip machine-local secrets from local files before they are pushed.
 
-    Two layers, both applied to EVERY collected file:
-
-    1. ``spec.sanitize_outbound_file`` -- the framework's own hook. It owns the
-       structural cleaning of the config files it knows about (hermes
-       ``config.yaml``, openhuman ``config.toml``, qwenpaw ``agent.json``, ...)
-       and fails closed (raises ``ValueError``) on one it cannot parse.
-    2. :func:`._secrets.redact_outbound` -- content-driven and
-       path-independent. The hook selects files by PATH, so a key an AI
-       assistant left in ``skills/*/scripts/*.py``, a skill-local ``mcp.json``,
-       ``SOUL.md`` or ``MEMORY.md`` used to be uploaded verbatim into the
-       remote repo and its git history (BUG-0909-01). This layer decides on
-       content instead, which also covers the frameworks that define no hook at
-       all (openclaw, nanobot, qoder).
-
-    *findings*, when given, receives one :class:`._secrets.Finding` per
-    redacted secret (never the secret text itself) so the caller can tell the
-    user what was stripped.
-
-    Accepts and returns the same ``{rel_path: bytes}`` mapping
-    ``collect_bytes`` produces (``str`` values are encoded as UTF-8, matching
-    the push path which uploads bytes). A file with nothing to redact keeps its
-    ORIGINAL bytes object: ``drop_unchanged_defaults`` compares bytes against
-    the framework default templates and ``push_mirror`` skips uploads by
-    sha256, so re-serializing an untouched file would defeat both.
+    Two layers, both applied to EVERY collected file: the framework's own
+    ``spec.sanitize_outbound_file`` hook, which owns the structural cleaning of
+    the config files it knows about and fails closed (``ValueError``) on one it
+    cannot parse; then :func:`._secrets.redact_outbound`, which decides on
+    content rather than path and so also covers ``skills/*``, the persona and
+    memory documents, and the frameworks with no hook at all (BUG-0909-01).
+
+    *findings*, when given, receives one :class:`._secrets.Finding` per redacted
+    secret (never the secret text) so the caller can report what was stripped.
+
+    Accepts and returns the ``{rel_path: bytes}`` mapping ``collect_bytes``
+    produces (``str`` values are encoded as UTF-8, matching the push path). A
+    file with nothing to redact keeps its ORIGINAL bytes object:
+    ``drop_unchanged_defaults`` and the sha256 push-skip both compare bytes.
     """
     out: dict = {}
     for rel, content in resources.items():
diff --git a/ms_agent/agent_hub/_watcher.py b/ms_agent/agent_hub/_watcher.py
index 7a1995680..3c3fa9ad7 100644
--- a/ms_agent/agent_hub/_watcher.py
+++ b/ms_agent/agent_hub/_watcher.py
@@ -159,7 +159,6 @@ def _poll_once(client, username, repo, framework, spec, push_only, state,
         sanitize_outbound(spec.collect_bytes(), spec, findings=redacted),
         framework, spec)
     if redacted:
-        # A Finding never carries the secret text, so this is safe to log.
         where = ', '.join(sorted({f.rel for f in redacted}))
         logger.warning('Redacted %d secret value(s) before push, in: %s',
                        len(redacted), where)
diff --git a/ms_agent/agent_hub/_workspace.py b/ms_agent/agent_hub/_workspace.py
index cc3e0179c..43da73c5a 100644
--- a/ms_agent/agent_hub/_workspace.py
+++ b/ms_agent/agent_hub/_workspace.py
@@ -606,8 +606,8 @@ def scrub_toml_secrets(text: str) -> str:
       secret-named query parameters are stripped.
 
     Shared by openhuman's ``config.toml`` and by the content-driven outbound
-    layer (:mod:`ms_agent.agent_hub._secrets`), which cleans ``.toml`` at ANY
-    collected path -- one secret vocabulary for both.
+    layer (:mod:`ms_agent.agent_hub._secrets`), which cleans ``.toml`` at any
+    collected path.
     """
     pattern = re.compile(
         r'^(?P
\s*(?P[A-Za-z0-9_.-]+)\s*=\s*)(?P.*)$')
@@ -1020,13 +1020,11 @@ def sanitize_outbound_file(self, rel_path: str, content: bytes) -> bytes:
         identity (qwenpaw ``agent.json``) must override this to blank secrets
         WITHOUT writing machine-local identity into the upload.
 
-        This hook is only the FIRST of two outbound layers, and it selects files
-        by PATH. :func:`._sync.sanitize_outbound` then runs every file through
-        the content-driven :func:`._secrets.redact_outbound`, which catches
-        secrets in the files no framework whitelists (``skills/*``, persona and
-        memory documents) -- so a framework that defines no hook at all is still
-        covered. Keep this hook for the structural cleaning and the fail-closed
-        refusals of the config files a framework owns.
+        This hook selects files by PATH and is only the first of two outbound
+        layers: :func:`._sync.sanitize_outbound` then runs every file through
+        the content-driven :func:`._secrets.redact_outbound`, so a framework
+        that defines no hook is still covered. Keep this hook for the structural
+        cleaning and fail-closed refusals of the configs a framework owns.
         """
         return self.sanitize_inbound_file(rel_path, content)
 
diff --git a/ms_agent/agent_hub/frameworks/openhuman.py b/ms_agent/agent_hub/frameworks/openhuman.py
index bd5bfd5aa..b3cd03435 100644
--- a/ms_agent/agent_hub/frameworks/openhuman.py
+++ b/ms_agent/agent_hub/frameworks/openhuman.py
@@ -311,13 +311,9 @@ def sanitize_inbound_file(self, rel_path: str, content: bytes) -> bytes:
         return self._scrub_toml_secrets(text).encode('utf-8')
 
     def _scrub_toml_secrets(self, text: str) -> str:
-        """Thin wrapper over the shared :func:`scrub_toml_secrets`.
-
-        The implementation moved to ``_workspace`` so the content-driven
-        outbound layer can clean ``.toml`` at ANY collected path with the same
-        rules (dotted keys, inline tables, ``[...env]`` / ``[...headers]``
-        sections, ``args`` arrays, multi-line strings, URL scalars).
-        """
+        """Wrapper over the shared :func:`scrub_toml_secrets`, which moved to
+        ``_workspace`` so the content-driven outbound layer can clean ``.toml``
+        at any collected path with the same rules."""
         return scrub_toml_secrets(text)
 
 
diff --git a/tests/agent_hub/test_cli.py b/tests/agent_hub/test_cli.py
index c20809826..bb945caa0 100644
--- a/tests/agent_hub/test_cli.py
+++ b/tests/agent_hub/test_cli.py
@@ -1767,9 +1767,7 @@ def test_upload_scrubs_ms_agent_settings_json_secrets(self):
         self.assertNotIn("sk-LEAKME333", raw)
         self.assertNotIn("env-LEAKME444", raw)
 
-    # BUG-0909-01: the framework hooks select files by PATH, so everything
-    # outside the root config whitelist (``skills/*``, the persona and memory
-    # documents) used to be uploaded verbatim.
+    # BUG-0909-01: skills/*, persona and memory files used to upload verbatim.
     B64_PERSONA_KEY = base64.b64encode(b"sk-S28PersonaB64Leak1").decode()
     SKILL_SENTINELS = (
         "sk-SkillScriptLeak01",
@@ -1837,8 +1835,7 @@ def test_upload_scrubs_skill_tree_and_persona(self):
             )
         self.assertEqual(rc, 0)
         client = _StubClient.instances[0]
-        # The skill tree really was collected -- otherwise the assertions
-        # below would pass vacuously.
+        # Guard against a vacuous pass: the skill tree really was collected.
         self.assertIn("skills/weather/scripts/leak_point.py",
                       client.uploaded_resources)
         self.assertIn("skills/weather/mcp.json", client.uploaded_resources)
@@ -1846,34 +1843,18 @@ def test_upload_scrubs_skill_tree_and_persona(self):
                          for v in client.uploaded_resources.values())
         for sentinel in self.SKILL_SENTINELS:
             self.assertNotIn(sentinel, blob)
-        # Benign documentation survives: the gate must not eat placeholders.
         self.assertIn('os.environ.get("OPENWEATHER_KEY", "")', blob)
         self.assertIn('os.environ["DASHSCOPE_API_KEY"]', blob)
         self.assertIn("--api-key ", blob)
         self.assertIn("Bearer $OPENAI_API_KEY", blob)
         self.assertIn("You are a helpful weather assistant.", blob)
-        # The user is told what was stripped -- without echoing the secret.
+        # The report names what was stripped without echoing the secret.
         report = buf.getvalue()
         self.assertIn("Secrets redacted", report)
         self.assertIn("skills/weather/scripts/leak_point.py", report)
         for sentinel in self.SKILL_SENTINELS:
             self.assertNotIn(sentinel, report)
 
-    @mock.patch("ms_agent.agent_hub._commands.AgentApi", _StubClient)
-    def test_dry_run_reports_redactions_without_uploading(self):
-        root = self._write_ws("ms-agent", self._skill_tree_files())
-        buf = io.StringIO()
-        with contextlib.redirect_stdout(buf):
-            rc = cmd_upload(
-                framework="ms-agent", name=None, local_dir=str(root),
-                dry_run=True, endpoint="http://s", token="tok", username="u",
-            )
-        self.assertEqual(rc, 0)
-        report = buf.getvalue()
-        self.assertIn("[dry-run] nothing uploaded.", report)
-        self.assertIn("Secrets redacted", report)
-        self.assertFalse(_StubClient.instances)
-
 
 class _OpenclawStub(_RepoStub):
     """Serves an openclaw single sub-agent repo (bare paths)."""
diff --git a/tests/agent_hub/test_secrets.py b/tests/agent_hub/test_secrets.py
index bbc5462fb..3c72ddb89 100644
--- a/tests/agent_hub/test_secrets.py
+++ b/tests/agent_hub/test_secrets.py
@@ -16,7 +16,6 @@
 
 SK_KEY = "sk-S26ProseLeak0001"
 GH_PAT = "ghp_S25bPatLeak7x9Qm2Rt4Vw"
-AWS_ID = "AKIAI3F0DNN7EXA1B2C4"
 JWT = ("eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0."
        "dOJvBm9vS2VyXzEyMzQ1Njc")
 B64_KEY = base64.b64encode(b"sk-S28B64Leak0001").decode()
@@ -43,14 +42,29 @@ def test_sk_key_in_python_assignment(self):
     def test_sk_key_in_shell_export(self):
         self._assert_redacted(f"export DASHSCOPE_API_KEY={SK_KEY}")
 
-    def test_github_pat(self):
-        out, hits = redact_text(f'GITHUB_TOKEN = "{GH_PAT}"')
-        self.assertNotIn(GH_PAT, out)
-        self.assertTrue(hits)
-
-    def test_aws_access_key_id(self):
-        out, _ = redact_text(f"aws_access_key_id = {AWS_ID}")
-        self.assertNotIn(AWS_ID, out)
+    def test_every_vendor_prefix_is_redacted(self):
+        """One case per branch of the vendor alternation."""
+        samples = (
+            ("openai", "sk-Pr0jK3yAbCdEfGhIjKlMn"),
+            ("anthropic", "sk-ant-api03-AbCdEfGhIjKlMnOpQr"),
+            ("github_pat", "ghp_S25bPatLeak7x9Qm2Rt4Vw"),
+            ("github_fine", "github_pat_11ABCDEFGH0XyZ9QwErTyUiOpAsDf"),
+            ("gitlab", "glpat-Xy7Zk2Mn9Pq4Rt6W"),
+            # Split literal: push protection rejects a contiguous Slack-token
+            # shape in source even when the fixture is fake.
+            ("slack", "xox" + "b-987654321098-AbCdEfGhIjKl"),
+            ("aws", "AKIAI3F0DNN7EXA1B2C4"),
+            ("google", "AIzaSyA1b2C3d4E5f6G7h8I9j0K"),
+            ("huggingface", "hf_XyZ1a2B3c4D5e6F7g8H9"),
+            ("npm", "npm_XyZ1a2B3c4D5e6F7g8H9"),
+            ("shopify", "shpat_XyZ1a2B3c4D5e6F7g8H9"),
+        )
+        for label, token in samples:
+            with self.subTest(vendor=label):
+                out, hits = redact_text(f"credential = {token}")
+                self.assertNotIn(token, out)
+                self.assertIn("[REDACTED:api_key]", out)
+                self.assertTrue(hits)
 
     def test_jwt(self):
         out, hits = redact_text(f"token: {JWT}")
@@ -91,27 +105,22 @@ def test_blob_decoding_to_a_key_is_redacted(self):
         self.assertIn("[REDACTED:base64]", out)
         self.assertEqual(hits[0][0], "base64")
 
-    def test_plain_base64_of_benign_text_is_kept(self):
-        blob = base64.b64encode(b"hello world, this is fine").decode()
-        text = f"echo {blob} | base64 -d"
-        out, hits = redact_text(text)
-        self.assertEqual(out, text)
-        self.assertEqual(hits, ())
-
-    def test_hex_digest_is_not_a_payload(self):
-        """A sha256 is base64-alphabet-shaped but decodes to binary."""
-        digest = "d4735e3a265e16eee03f59718b9b5d03019c07d8b6c51f90da3a666eec13ab35"
-        text = f"commit {digest} is the release"
-        out, hits = redact_text(text)
-        self.assertEqual(out, text)
-        self.assertEqual(hits, ())
-
-    def test_data_uri_is_skipped(self):
-        payload = base64.b64encode(b"PNGDATA" * 12).decode()
-        text = f"![img](data:image/png;base64,{payload})"
-        out, hits = redact_text(text)
-        self.assertEqual(out, text)
-        self.assertEqual(hits, ())
+    def test_non_secret_blobs_are_kept(self):
+        """Benign text, a hex digest (decodes to binary) and a data URI."""
+        benign = base64.b64encode(b"hello world, this is fine").decode()
+        digest = ("d4735e3a265e16eee03f59718b9b5d03"
+                  "019c07d8b6c51f90da3a666eec13ab35")
+        png = base64.b64encode(b"PNGDATA" * 12).decode()
+        cases = (
+            f"echo {benign} | base64 -d",
+            f"commit {digest} is the release",
+            f"![img](data:image/png;base64,{png})",
+        )
+        for text in cases:
+            with self.subTest(text=text[:40]):
+                out, hits = redact_text(text)
+                self.assertEqual(out, text)
+                self.assertEqual(hits, ())
 
 
 class TestDocumentedPlaceholdersSurvive(unittest.TestCase):
@@ -163,12 +172,6 @@ def test_clean_content_is_untouched(self):
                 self.assertEqual(out, text)
                 self.assertEqual(hits, ())
 
-    def test_clean_multiline_document_round_trips(self):
-        doc = "\n".join(self.CLEAN)
-        out, hits = redact_text(doc)
-        self.assertEqual(out, doc)
-        self.assertEqual(hits, ())
-
 
 class TestNameStrength(unittest.TestCase):
     """Which key names may drive a redaction, and how hard they push."""
@@ -239,17 +242,14 @@ def test_bare_userinfo_token_is_redacted(self):
         out, _ = redact_text("git clone https://ghp_AbCdEfG7h9IjK2LmNo@h/r")
         self.assertNotIn("ghp_AbCdEfG7h9IjK2LmNo", out)
 
-    def test_documented_example_url_survives(self):
-        text = "postgresql+asyncpg://user:pass@localhost/db"
-        out, hits = redact_text(text)
-        self.assertEqual(out, text)
-        self.assertEqual(hits, ())
-
-    def test_variable_query_value_survives(self):
-        text = "https://gateway.example.com/v1?access_token=${token}"
-        out, hits = redact_text(text)
-        self.assertEqual(out, text)
-        self.assertEqual(hits, ())
+    def test_documented_urls_survive(self):
+        """A doc example password and a variable-reference query value."""
+        for text in ("postgresql+asyncpg://user:pass@localhost/db",
+                     "https://gateway.example.com/v1?access_token=${token}"):
+            with self.subTest(text=text):
+                out, hits = redact_text(text)
+                self.assertEqual(out, text)
+                self.assertEqual(hits, ())
 
     def test_trailing_punctuation_is_not_swallowed(self):
         out, _ = redact_text(
@@ -418,11 +418,6 @@ def test_findings_never_carry_the_secret(self):
             for field in finding:
                 self.assertNotIn(SK_KEY, str(field))
 
-    def test_finding_line_numbers_are_one_based(self):
-        text = "line one\nline two\napi_key = %s\n" % SK_KEY
-        _out, hits = redact_text(text)
-        self.assertEqual(hits[0][1], 3)
-
     def test_jsonl_history_keeps_cursors_and_drops_keys(self):
         """nanobot ``memory/history.jsonl`` mixes pagination cursors with
         credentials; only the credential may go."""
diff --git a/tests/agent_hub/test_workspace.py b/tests/agent_hub/test_workspace.py
index 4635a573a..e79dc177e 100644
--- a/tests/agent_hub/test_workspace.py
+++ b/tests/agent_hub/test_workspace.py
@@ -1609,14 +1609,9 @@ def test_toml_single_quoted_url(self):
 class TestOutboundCoverageAcrossFrameworks(unittest.TestCase):
     """BUG-0909-01: outbound cleaning must be content-driven, not path-driven.
 
-    Every framework collects ``skills/*`` recursively plus its persona and
-    memory documents, but the per-framework ``sanitize_outbound_file`` hooks
-    select files by PATH: ms-agent whitelisted ``settings.json`` / ``mcp.json``,
-    qwenpaw only ``agent.json``, hermes / openhuman only their root config, and
-    openclaw / nanobot / qoder defined no hook at all. A key an AI assistant
-    wrote into a skill script, a skill-local ``mcp.json``, ``SOUL.md`` or a
-    memory file was therefore uploaded verbatim into the remote repo and its
-    git history.
+    Every framework collects ``skills/*`` plus its persona and memory
+    documents, and three of them defined no outbound hook at all, so the fix
+    has to hold for all seven rather than for the two the bug report named.
     """
 
     B64_KEY = base64.b64encode(b"sk-PersonaB64Leak001").decode()
@@ -1647,8 +1642,7 @@ class TestOutboundCoverageAcrossFrameworks(unittest.TestCase):
         "nanobot": "SOUL.md",
         "qoder": "AGENTS.md",
     }
-    # The memory slot, where the framework has one (ms-agent keeps memory
-    # project-level, so its global home carries none).
+    # The memory slot; ms-agent keeps memory project-level, so it has none.
     MEMORY = {
         "qwenpaw": "MEMORY.md",
         "openclaw": "MEMORY.md",
@@ -1734,10 +1728,10 @@ def _outbound(self, framework):
         return files, collected, sanitize_outbound(collected, spec,
                                                    findings=findings), findings
 
-    def test_every_framework_redacts_skills_persona_and_memory(self):
+    def test_every_framework_redacts_secrets_but_keeps_documentation(self):
         for framework in sorted(FRAMEWORK_REGISTRY):
             with self.subTest(framework=framework):
-                files, collected, out, findings = self._outbound(framework)
+                _files, collected, out, findings = self._outbound(framework)
                 # Guard against a vacuous pass: the carriers really were
                 # collected by this framework's patterns.
                 self.assertIn(self.PERSONA[framework], collected)
@@ -1749,21 +1743,16 @@ def test_every_framework_redacts_skills_persona_and_memory(self):
                             and framework not in self.MEMORY:
                         continue
                     self.assertNotIn(sentinel, blob)
+                # Over-redaction is this layer's likely failure mode, so the
+                # same pass pins the documentation that must survive.
+                for needle in self.BENIGN:
+                    self.assertIn(needle, blob)
                 self.assertTrue(findings, "no redaction reported")
                 for finding in findings:
                     for field in finding:
                         for sentinel in self.SENTINELS:
                             self.assertNotIn(sentinel, str(field))
 
-    def test_benign_documentation_survives_in_every_framework(self):
-        for framework in sorted(FRAMEWORK_REGISTRY):
-            with self.subTest(framework=framework):
-                _files, _collected, out, _findings = self._outbound(framework)
-                blob = "\n".join(v.decode("utf-8", "replace")
-                                 for v in out.values())
-                for needle in self.BENIGN:
-                    self.assertIn(needle, blob)
-
     def test_clean_workspace_is_not_rewritten(self):
         """Byte identity matters: ``drop_unchanged_defaults`` compares bytes and
         ``push_mirror`` skips uploads by sha256."""