T09 · Insecure Skill Coding Practices
Error
- Location
- ` or the corresponding apply command. 3. The path `/channels/feishu/accounts/example` passes the mutation allowlist. 4. `mask_value()` examines only that path and finds no secret keyword. 5. The complete `value` object, including `appSecret`, is placed in the `applied` report. 6. The secret is printed in the execution receipt. 7. The raw proposal is additionally stored at `/tmp/oc-guard-last-proposal.json`. 8. Anyone with access to captured output or the temporary artifact may recover the credential. The s ...[truncated 1034 chars]:104
- Finding
- Nested configuration secrets are disclosed in receipts and temporary proposal files<![CDATA[ ## Vulnerability Details **File Location**: `scripts/oc-guard.py:46`, `scripts/oc-guard.py:104-109`, `scripts/oc-guard.py:627-650`, `scripts/oc-guard.py:728-756`, and `scripts/oc-guard.py:758-824` **Vulnerability Type**: Inadequate recursive secret redaction and plaintext sensitive-data storage **Risk Level**: High ### Vulnerable Code ```python SECRET_RE = re.compile(r"(secret|token|apikey|api_key|password)", re.IGNORECASE) ``` ```python def mask_value(path: str, value): if SECRET_RE.search(path): s = str(value) if len(s) <= 8: return "****" return s[:4] + "****" + s[-4:] return value ``` The masking function is invoked using only the top-level mutation path: ```python def apply_changes(cfg, proposal): changes = proposal.get("changes") if not isinstance(changes, list) or not changes: raise ValueError("proposal.changes must be non-empty list") modified = copy.deepcopy(cfg) applied = [] risks = [] for c in changes: if not isinstance(c, dict): raise ValueError("each change must be object") op = c.get("op", "set") path = c.get("path") if not isinstance(path, str): raise ValueError("change.path must be string") if not path.startswith(ALLOWED_PATH_PREFIXES): raise ValueError(f"path not allowed by policy: {path}") if op == "set": if "value" not in c: raise ValueError(f"missing value for set: {path}") set_path(modified, path, c["value"]) shown = mask_value(path, c["value"]) applied.append({"op": op, "path": path, "value": shown}) elif op == "delete": delete_path(modified, path) applied.append({"op": op, "path": path}) else: raise ValueError(f"unsupported op: {op}") risks.append(risk_of_path(path)) return modified, applied, merge_risk(risks) ``` The resulting value is pri ...[truncated 3289 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Replace path-only masking with recursive redaction of dictionaries and lists. ```python def redact_value(value, key_path=""): if isinstance(value, dict): result = {} for key, child in value.items(): child_path = f"{key_path}/{key}" if SECRET_RE.search(str(key)): result[key] = "****" else: result[key] = redact_value(child, child_path) return result if isinstance(value, list): return [redact_value(item, key_path) for item in value] if SECRET_RE.search(key_path): return "****" return value ``` 2. Apply recursive redaction before adding values to receipt data: ```python shown = redact_value(c["value"], path) ``` 3. Do not persist raw proposals when they may contain credentials. Persist only a redacted copy unless the original is strictly required for execution. 4. If raw persistence is unavoidable: - Store the file in a private, user-owned directory. - Set directory permissions to `0700`. - Set file permissions to `0600`. - Delete the artifact as soon as it is no longer required. - Document its sensitive nature clearly. 5. Extend secret detection to normalize key names and cover common variants such as `authorization`, `credential`, `privateKey`, `clientSecret`, and `accessKey`. 6. Add regression tests for: - A direct secret path. - A parent-object mutation containing `appSecret`. - Nested dictionaries containing `botToken`. - Lists containing credential objects. - Plan and apply receipt output. - Persisted proposal artifacts. ]]>