Skip to content

Cowork security follow-ups — gate on promoting cowork out of experimental #925

Description

@sergio-sisternes-epam

Follow-up to #913 (Cowork skills experimental support). Surfaced by the APM Expert Review Panel (Supply Chain Security specialist). CEO arbitration ruled these acceptable as follow-ups behind the experimental flag, but hard gates on exiting experimental — the cowork flag must not be promoted until all three are resolved.

1. Stale cowork files never cleaned up (BLOCKING for GA)

File: src/apm_cli/integration/cleanup.py:126

remove_stale_deployed_files() constructs project_root / stale_path. For a cowork://skills/<name> entry this produces a non-existent path under the project root; the helper treats it as "already gone" and skips. The real file at ~/Library/CloudStorage/.../Documents/Cowork/skills/<name>/SKILL.md persists. Compare with BaseIntegrator.sync_remove_files() (base_integrator.py:413-425) which correctly resolves cowork:// via from_lockfile_path.

Threat model: persistence of attacker-controlled AI instructions in a user's M365 Copilot environment after APM package removal. Mitigated today by the experimental flag (explicit opt-in, detect_by_dir=False, limited blast radius).

Fix:

  • Mirror the cowork:// resolution branch from base_integrator.py:413-425 in cleanup.py.
  • When calling cleanup_empty_parents for a resolved cowork path, pass stop_at=cowork_root — today it walks to /.

2. set_cowork_skills_dir missing validate_path_segments + NUL-byte check

File: src/apm_cli/config.py:156-175

os.path.isabs('/foo\x00bar') returns True; Path(...).resolve() then raises ValueError which escapes the current except PathTraversalError block.

Fix:

  • Call validate_path_segments(value, context="cowork-skills-dir") before Path.resolve().
  • Explicit NUL-byte check ("\x00" in value) with a clear error message.
  • Extend the existing except to also cover ValueError from resolve().

3. Ad-hoc ".." in rel_path substring check

File: src/apm_cli/integration/base_integrator.py:142

Substring match rejects legitimate filenames such as foo..bar.md. Pattern violation — repo convention is validate_path_segments.

Fix: replace the substring check with validate_path_segments(rel_path.parts, context="cowork deploy path").


Acceptance criteria

  • cleanup.py resolves cowork:// prefixes via from_lockfile_path and removes the real file.
  • cleanup_empty_parents stops at cowork_root for cowork paths, not /.
  • set_cowork_skills_dir rejects NUL bytes and traversal segments with a clear, actionable error.
  • base_integrator.py:142 uses validate_path_segments instead of substring matching.
  • Unit tests cover: stale-file removal on OneDrive-like path, NUL-byte rejection, foo..bar.md accepted, traversal segments rejected.
  • Full unit suite green (uv run pytest tests/unit tests/test_console.py -x).

Release gate

The cowork experimental flag must not be promoted to GA (removed from FLAGS in src/apm_cli/core/experimental.py) until this issue is closed. The security model doc should be refreshed at the same time since cowork writes into a surface ingested by M365 Copilot.

/cc panel review: #913

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/multi-targetMulti-target deploy spec, target directory creation, agent surface routing.experimentalsecurityDeprecated: use theme/security. Kept for issue history; will be removed in milestone 0.10.0.

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    Status
    Done

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions