fix(policy): handle OSError from is_file() on macOS PATH_MAX limit

[chaobo8484]

When load_policy() receives a YAML string that exceeds ~1023 bytes, the is_file() syscall fails on macOS (PATH_MAX ≈ 1024) with OSError [Errno 63] File name too long. Wrap the call in try/except to gracefully fall back to treating the input as a YAML string.

Fixes #848

Description

Brief description of changes and motivation.

Fixes # (issue)

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

Fixes load_policy() to avoid crashing on macOS when given a long YAML string that triggers Path.is_file() to raise OSError: [Errno 63] File name too long, preserving the intended "path or YAML content" input behavior.

Changes:

• Wraps the Path.is_file() probe in try/except OSError and falls back to treating the input as YAML content when the probe fails.

Show a summary per file

File	Description
src/apm_cli/policy/parser.py	Adds defensive handling around Path.is_file() so long YAML inputs do not raise on macOS PATH_MAX limits.

Copilot's findings

• Files reviewed: 1/1 changed files
• Comments generated: 2

[danielmeppiel]

Per Copilot reviewer

[chaobo8484]

@danielmeppiel I have made adjustments according to Copilot's suggestion.Have a good day！

[github-actions]

DevX UX Review — PR #860

Scope: CLI error ergonomics & silent-failure risk. No command surface / flag / help-text changes in this PR — review is limited to internal load_policy() behaviour and its downstream user-visible effects.

---

Findings

1. ✅ Error recovery is solid. When ENAMETOOLONG fires, the code falls back to treating the input as YAML. If that YAML is malformed, yaml.safe_load raises YAMLError, which is wrapped into a PolicyValidationError with a human-readable message. The user still gets a clear "YAML parse error: ..." diagnostic — no silent swallowing of bad input.

2. ✅ _looks_like_yaml_content() is a reasonable fast-path. The heuristics (newlines, ---, - , {, [, : ) match what every YAML-literate developer would expect. This avoids the filesystem probe entirely for the vast majority of inline YAML, which is the right default. No UX concern here.

3. ⚠️ Minor: a valid-looking file path that doesn't exist is silently parsed as YAML. This is pre-existing behaviour, not introduced by this PR, but worth noting: load_policy("apm-policy.yml") where the file is missing will try to YAML-parse the string "apm-policy.yml" and return {name: apm-policy.yml} (bare YAML scalar coerced to a mapping? — actually it would hit "Policy must be a YAML mapping"). So the failure mode is noisy enough. No new risk from this PR.

4. ✅ No user-facing output changes. apm audit --policy <path> UX is unaffected — callers in discovery.py already pass either a Path object or pre-read content strings. The fix only matters when raw YAML strings happen to exceed PATH_MAX, which is a programmatic edge case (not a CLI-surface issue).

5. ✅ Test coverage is appropriate. The new test constructs a >1 KiB YAML payload and asserts it parses without OSError. Good enough for a defensive fix — no over-engineering.

Verdict: Ship it. Clean defensive fix, no UX regressions, no silent failures introduced. 👍

Generated by PR Review Panel for issue #860 · ● 4M · ◷

[github-actions]

APM Review Panel Verdict

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

---

Per-persona findings

Python Architect: Routine bug-fix PR. One helper extracted (_looks_like_yaml_content), one function refactored (load_policy). Two mermaid diagrams follow.

OO / class diagram

classDiagram
    class ApmPolicy {
        <<dataclass>>
        +name: str
        +version: str
        +enforcement: str
    }
    class load_policy:::touched {
        <<function>>
        +source: Union[str, Path]
        +returns: Tuple[ApmPolicy, List[str]]
    }
    class _looks_like_yaml_content:::touched {
        <<function>>
        +source: str
        +returns: bool
    }
    class _build_policy {
        <<function>>
        +data: dict
        +returns: ApmPolicy
    }
    load_policy:::touched ..> _looks_like_yaml_content:::touched : calls (new guard)
    load_policy:::touched ..> _build_policy : calls
    _build_policy ..> ApmPolicy : creates
    note for _looks_like_yaml_content "Guard: avoids Path.is_file() on large\nstrings (macOS ENAMETOOLONG fix)"
    note for load_policy "Fixed: OSError ENAMETOOLONG caught\nand treated as is_file=False"

Loading

Execution-flow diagram

flowchart TD
    A["load_policy(source)"] --> B{"isinstance(source, Path)?"}
    B -- Yes --> C["source.read_text() if is_file else str(source)"]
    B -- No --> D{"_looks_like_yaml_content(source)?<br/>(newline, YAML marker, key: val)"}
    D -- Yes --> E["raw = source (skip filesystem probe)"]
    D -- No --> F["path = Path(source)"]
    F --> G["try: path.is_file()"]
    G -- "OSError ENAMETOOLONG" --> H["is_file = False"]
    G -- "other OSError" --> I["re-raise"]
    G -- success --> J{"is_file?"}
    J -- Yes --> K["raw = path.read_text()"]
    J -- No --> L["raw = source"]
    H --> L
    E --> M["yaml.safe_load(raw)"]
    K --> M
    L --> M
    M --> N["_build_policy(data)"]
    N --> O["return ApmPolicy, warnings"]

Loading

Design patterns: Guard clause (_looks_like_yaml_content short-circuits before filesystem IO), defensive programming (specific errno.ENAMETOOLONG catch, re-raise on all others), single-responsibility extraction (heuristic is its own named, independently testable function). All patterns are correct and proportionate to the fix scope.

Coverage gap (not a blocker): The test exercises only the _looks_like_yaml_content() fast path. The yaml_str contains \n (the trailing newline of long_comment), so "\n" in source returns True and Path.is_file() is never called. The errno.ENAMETOOLONG catch branch has no explicit coverage.

---

CLI Logging Expert: No output, logging, CommandLogger, DiagnosticCollector, or STATUS_SYMBOLS changes. Non-ENAMETOOLONG OSError exceptions are re-raised, preserving the existing failure signal path. No concerns.

---

DevX UX Expert: No CLI command surface, flag, help text, or user-facing error message changes. The fix is transparent to users (silent crash eliminated on macOS). No documentation update required -- no command surface changed. No UX regression.

---

Supply Chain Security Expert: _looks_like_yaml_content() determines whether a string is treated as a file path or inline YAML. Misclassifying a file path as YAML prevents a file read rather than enabling one -- security-neutral at worst. yaml.safe_load() is unchanged; no arbitrary-object deserialization risk. No new path traversal surface (no new Path() construction from user-controlled strings). Fixing the crash is security-positive: an unhandled ENAMETOOLONG could cause policy loading to abort entirely, potentially triggering fail-open behavior upstream. No concerns.

---

Auth Expert: Not activated -- changed files (src/apm_cli/policy/parser.py, tests/unit/policy/test_parser.py) are in the policy parsing subsystem with no auth, token, credential, host-classification, or AuthResolver semantics.

---

OSS Growth Hacker: macOS is the primary developer platform for APM's target audience. A crash when loading large inline policy strings is a trust regression that disproportionately hits enterprise users (who are more likely to have complex, multi-line policies). The fix is correct and the test is appropriate. One missing item: no CHANGELOG entry. Per repo convention, every code change needs a Fixed line under ## [Unreleased]. Side-channel to CEO: this is a 1-line add that protects the quality signal in release notes and demonstrates active platform parity maintenance to the community.

---

CEO arbitration

The panel is unanimous and the findings are clean. This is a targeted, non-breaking, correct bug fix for a real macOS platform crash. There are no architectural concerns, no security regressions, no UX changes, and no auth implications. The only gap is the missing CHANGELOG.md entry -- a repo convention requirement, not a nice-to-have -- and an optional (non-blocking) coverage gap for the errno.ENAMETOOLONG catch branch. I ratify APPROVE with one required pre-merge action.

---

Required actions before merge

1. Add a Fixed entry to CHANGELOG.md under ## [Unreleased] -> ### Fixed. Suggested line:

   - `load_policy()` no longer raises `OSError: [Errno 63] ENAMETOOLONG` on macOS when called with large inline YAML strings longer than PATH_MAX. (#860)
   

---

Optional follow-ups

• Add an explicit unit test for the errno.ENAMETOOLONG branch by patching Path.is_file to raise OSError(errno.ENAMETOOLONG, "File name too long") -- the current test only exercises the _looks_like_yaml_content() fast path (newline check).
• Audit other Path.is_file() / Path.exists() call sites in src/apm_cli/policy/discovery.py for the same macOS ENAMETOOLONG exposure if they can receive user-supplied strings.

Generated by PR Review Panel for issue #860 · ● 510.3K · ◷