Skip to content

refactor(policy): consolidate duplicated YAML struct hierarchies#97

Merged
johntmyers merged 2 commits intomainfrom
refactor/96-consolidate-policy-yaml-structs/jomyers
Mar 4, 2026
Merged

refactor(policy): consolidate duplicated YAML struct hierarchies#97
johntmyers merged 2 commits intomainfrom
refactor/96-consolidate-policy-yaml-structs/jomyers

Conversation

@johntmyers
Copy link
Copy Markdown
Collaborator

🏗️ build-from-issue-agent

Closes #96

Summary

Consolidate the Deserialize-only input structs and Serialize-only output structs into a single set of types in navigator-policy that derive both Serialize and Deserialize. This eliminates the duplicate PolicyYaml hierarchy in navigator-cli and fixes three round-trip bugs where ncl sandbox policy get --full output could not be fed back as input.

Changes Made

  • crates/navigator-policy/src/lib.rs: Added Serialize derive to all YAML serde structs, added skip_serializing_if attributes for clean output, switched HashMap to BTreeMap for deterministic ordering, added api_patterns field to InferenceDef, added InferenceApiPatternDef struct, added pub fn serialize_sandbox_policy() for proto-to-YAML conversion, renamed convert_policy() to to_proto(), added from_proto() for the reverse direction, added 4 round-trip tests
  • crates/navigator-cli/src/run.rs: Deleted 10 duplicate *Yaml structs (~85 lines) and policy_to_yaml() function (~77 lines), replaced both call sites with navigator_policy::serialize_sandbox_policy(), removed unused use serde::Serialize import

Deviations from Plan

None — implemented as planned.

Tests Added

  • Unit: 4 tests in navigator-policy/src/lib.rs:
    • round_trip_default_policy — full round-trip of dev-sandbox-policy.yaml
    • serialized_yaml_uses_filesystem_policy_key — verifies canonical field name
    • round_trip_preserves_allowed_ips — verifies no data loss on allowed_ips
    • round_trip_preserves_policy_name — verifies no data loss on policy name field
    • round_trip_preserves_api_patterns — verifies api_patterns survives round-trip
  • Integration: N/A
  • E2E: N/A — no e2e/ files modified

Documentation Updated

  • None needed — architecture/security-policy.md already uses the correct canonical field names

Verification

  • Pre-commit checks passing (unit tests, lint, format) — license check failure is pre-existing (tmp/network_checks.py), unrelated
  • No e2e/ files modified — E2E phase skipped
  • All 363 Rust tests passing across the workspace

… navigator-policy

Closes #96

Merge the Deserialize-only input structs and Serialize-only output structs
into a single set of types in navigator-policy that derive both Serialize
and Deserialize. This eliminates the duplicate PolicyYaml hierarchy in
navigator-cli and fixes three round-trip issues:

- filesystem_policy vs filesystem field name mismatch
- allowed_ips silently dropped on proto-to-YAML conversion
- network policy name field silently dropped on proto-to-YAML conversion

Also adds api_patterns support to the inference YAML schema and switches
network_policies from HashMap to BTreeMap for deterministic output ordering.
Align test_l4_non_connect_method_rejected with the proxy behavior
change in 52b48d1 which intentionally returns 403 for non-CONNECT
requests.
@johntmyers johntmyers requested review from drew and pimlock March 4, 2026 18:33
@johntmyers johntmyers merged commit 6079c66 into main Mar 4, 2026
9 checks passed
@johntmyers johntmyers deleted the refactor/96-consolidate-policy-yaml-structs/jomyers branch March 4, 2026 18:35
drew pushed a commit that referenced this pull request Mar 16, 2026
* refactor(policy): consolidate duplicated YAML struct hierarchies into navigator-policy

Closes #96

Merge the Deserialize-only input structs and Serialize-only output structs
into a single set of types in navigator-policy that derive both Serialize
and Deserialize. This eliminates the duplicate PolicyYaml hierarchy in
navigator-cli and fixes three round-trip issues:

- filesystem_policy vs filesystem field name mismatch
- allowed_ips silently dropped on proto-to-YAML conversion
- network policy name field silently dropped on proto-to-YAML conversion

Also adds api_patterns support to the inference YAML schema and switches
network_policies from HashMap to BTreeMap for deterministic output ordering.

* fix(e2e): update non-CONNECT test assertion from 405 to 403

Align test_l4_non_connect_method_rejected with the proxy behavior
change in c06117e which intentionally returns 403 for non-CONNECT
requests.

---------

Co-authored-by: John Myers <johntmyers@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor: consolidate duplicated policy YAML struct hierarchies in navigator-policy

1 participant