fix: rename token variable to resolve CodeQL clear-text-storage alert

[Copilot]

Description

CodeQL py/clear-text-storage-sensitive-data flags _substitute_plugin_root in plugin_parser.py because a local variable named token holds the template placeholder "${CLAUDE_PLUGIN_ROOT}". The heuristic treats any variable named token as sensitive data. False positive — renamed to placeholder.

# Before
token = "${CLAUDE_PLUGIN_ROOT}"
if isinstance(obj, str) and token in obj:
    return obj.replace(token, abs_root)

# After
placeholder = "${CLAUDE_PLUGIN_ROOT}"
if isinstance(obj, str) and placeholder in obj:
    return obj.replace(placeholder, abs_root)

Type of change

• Bug fix
• New feature
• Documentation
• Maintenance / refactor

Testing

• Tested locally
• All existing tests pass
• Added tests for new functionality (if applicable)

[Copilot]

Pull request overview

This PR addresses a CodeQL false-positive (py/clear-text-storage-sensitive-data) by renaming a local variable in the plugin root placeholder substitution logic so it is no longer named token.

Changes:

• Rename the local variable holding "${CLAUDE_PLUGIN_ROOT}" from token to placeholder in _substitute_plugin_root.
• Update uv.lock to reflect apm-cli version 0.10.0.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.

File	Description
uv.lock	Updates the locked editable package version for apm-cli to 0.10.0.
src/apm_cli/deps/plugin_parser.py	Renames the placeholder variable in ${CLAUDE_PLUGIN_ROOT} substitution to avoid CodeQL heuristic false positives.

[github-actions]

APM Review Panel Verdict

Disposition: APPROVE (with one minor pre-merge action -- add CHANGELOG entry)

---

Per-persona findings

Python Architect:

This is a routine PR (single variable rename inside one function). Two mermaid blocks follow.

1. OO / class diagram

The PR participates in deps/plugin_parser.py, a module of pure helper functions for reading and transforming MCP server configs. No class hierarchy exists here -- all functions are procedural. The diagram shows module boundaries and function entry points.

classDiagram
    direction LR
    class plugin_parser {
        <<Module>>
        +_read_mcp_json(path, logger) Dict
        +_substitute_plugin_root(servers, abs_root, logger) Dict
        +_mcp_servers_to_apm_deps(servers, plugin_path) List
    }
    class _substitute_plugin_root {
        <<Pure>>
        placeholder str
        substituted bool
        +_walk(obj) Any
    }
    class PluginParserCaller {
        <<IOBoundary>>
    }
    plugin_parser *-- _substitute_plugin_root : contains
    PluginParserCaller ..> plugin_parser : calls
    class _substitute_plugin_root:::touched
    classDef touched fill:#fff3b0,stroke:#d47600

Loading

2. Execution flow diagram

flowchart TD
    A["PluginParser caller"] --> B["_substitute_plugin_root(servers, abs_root, logger)"]
    B --> C["Initialize placeholder = '\$\{CLAUDE_PLUGIN_ROOT}'"]
    C --> D["_walk(obj) -- recursive traversal"]
    D --> E{isinstance obj str?}
    E -- yes --> F{placeholder in obj?}
    F -- yes --> G["substituted=True\nreturn obj.replace(placeholder, abs_root)"]
    F -- no --> H["return obj unchanged"]
    E -- no --> I{isinstance obj dict?}
    I -- yes --> J["recurse over k,v pairs"] --> D
    I -- no --> K{isinstance obj list?}
    K -- yes --> L["recurse over items"] --> D
    K -- no --> M["return obj"]
    G --> N["[I/O] logger.info: substitution logged"]

Loading

Design patterns

• Used in this PR: none -- straight-line procedural code with a recursive walk helper; appropriate for the scope.
• Pragmatic suggestion: none -- the current shape is the simplest correct design at this scope.

Assessment: The rename from token to placeholder is correct and semantically clearer ("\$\{CLAUDE_PLUGIN_ROOT}" IS a template placeholder, not an auth token). The uv.lock version sync (0.9.4 -> 0.10.0) corrects a stale lock file -- pyproject.toml already declared 0.10.0 on main. No architectural concerns.

---

CLI Logging Expert: No logging changes in this PR. The logger.info("Substituted \$\{CLAUDE_PLUGIN_ROOT} with %s", abs_root) call at line 295 is untouched. No _rich_* helpers, no CommandLogger or DiagnosticCollector changes. The function correctly uses logging.Logger (appropriate for an internal utility, not a command handler). No issues.

---

DevX UX Expert: plugin_parser.py is an internal deps/ module with no CLI surface exposure. No commands, flags, help text, error messages, or user-facing flows are touched. This fix is completely invisible to end users. No documentation update needed (no CLI surface changed). No issues.

---

Supply Chain Security Expert: The CodeQL py/clear-text-storage-sensitive-data alert was a genuine false positive. The value "\$\{CLAUDE_PLUGIN_ROOT}" is a deterministic path template marker, not a credential -- the analyzer triggered on the variable name token alone. The fix is correct: placeholder removes the false positive without any behavioral change.

uv.lock change: only the apm-cli editable package version string changes (0.9.4 -> 0.10.0). No new transitive dependencies, no hash modifications. The path _substitute_plugin_root operates entirely in-memory (string transformation on dicts); it does not write files or construct filesystem paths for I/O, so path_security guards are not applicable here. No supply chain surface opened.

---

Auth Expert: Not activated -- src/apm_cli/deps/plugin_parser.py only renames a local variable holding a path template string; no authentication, token management, credential resolution, or host classification flow is affected.

---

OSS Growth Hacker: No conversion surface touched (no README, quickstart, CLI surface, templates, or docs changes). Keeping CodeQL clean is a positive hygiene signal for enterprise adopters evaluating security posture. No growth strategy update needed for WIP/growth-strategy.md. Side-channel to CEO: single missing artifact is a CHANGELOG.md [Unreleased] > Fixed entry -- the repo convention requires one for every code-touching PR, and this one has none.

---

CEO arbitration

All five specialists agree with no conflicts to resolve. This is a textbook false-positive suppression fix: a variable whose name tripped a static-analysis heuristic is renamed without any behavioral change. The uv.lock version sync is legitimate housekeeping (the lock file was stale at 0.9.4 while pyproject.toml already declared 0.10.0 on main after the last release cut). The change is clean, minimal, and appropriate. The one missing artifact is a CHANGELOG.md entry -- the repo convention mandates one for every code-touching PR. APPROVE once that entry is added.

---

Required actions before merge

1. Add a CHANGELOG.md entry under ## [Unreleased] > ### Fixed. Suggested text: Fixed CodeQL \py/clear-text-storage-sensitive-data` false positive in `_substitute_plugin_root` -- renamed local variable from `token` to `placeholder` (no behavior change). (fix: rename token variable to resolve CodeQL clear-text-storage alert #1002)`

---

Optional follow-ups

• The uv.lock version was stale on main (0.9.4 vs pyproject.toml 0.10.0) until this PR fixed it. Consider adding a CI lint step or release checklist item to assert that the apm-cli version in uv.lock matches pyproject.toml before a release is cut -- catches drift early.
• _substitute_plugin_root has no dedicated unit tests. The variable rename carries no behavioral risk, but test coverage for the recursive walk logic (dict, list, nested, no-op, and partial-match cases) would protect future refactors.

Generated by PR Review Panel for issue #1002 · ● 750.6K · ◷