Skip to content

fix: warn when unsupported L7 access presets are ignored #2008

Description

@pimlock

Description

L7 endpoint access presets that the proxy does not recognize are currently accepted by policy parsing but ignored during proxy-side expansion. For example, the PR 1638 SigV4 docs review caught access: allow: it parses, but expand_access_presets only expands read-only, read-write, and full, so the _ branch silently skips it.

That silent skip makes failures hard to diagnose. A user may believe they granted access to an endpoint, but requests still do not work, and the logs do not explain that the requested access value was ignored.

Context

Agent Investigation

Skill loaded: create-github-issue.

Findings:

  • validate_l7_policies checks that rules and access are mutually exclusive, and that an L7 endpoint has either rules or access, but it does not validate that the access string is one of the supported presets. See crates/openshell-supervisor-network/src/l7/mod.rs:587 and crates/openshell-supervisor-network/src/l7/mod.rs:592.
  • expand_access_presets is the proxy-side expansion point. It expands read-only, read-write, and full for graphql, websocket, and other REST-like protocols. For any other value, including allow, the match arm uses _ => continue, leaving no expanded rules and emitting no warning. See crates/openshell-supervisor-network/src/l7/mod.rs:1067 and the fallthroughs at crates/openshell-supervisor-network/src/l7/mod.rs:1109, crates/openshell-supervisor-network/src/l7/mod.rs:1116, and crates/openshell-supervisor-network/src/l7/mod.rs:1134.
  • There is a related validation path in crates/openshell-policy/src/merge.rs: expand_existing_access returns PolicyMergeError::UnsupportedAccessPreset when policy merge/update code tries to expand an unsupported preset. That does not cover the supervisor-network JSON expansion path that the proxy uses when loading policy data. See crates/openshell-policy/src/merge.rs:700.
  • Nearby unit tests already cover accepted presets and validation warnings in crates/openshell-supervisor-network/src/l7/mod.rs, so a focused regression test can live beside expand_read_only_preset / expand_full_preset. See crates/openshell-supervisor-network/src/l7/mod.rs:1620.

What was tried:

  • Inspected the PR review comment and confirmed the reported bad value was access: allow.
  • Searched for existing unsupported-access handling across crates/openshell-supervisor-network, crates/openshell-policy, and docs.
  • Ran cargo test -p openshell-supervisor-network l7::tests::expand_read_only_preset to validate the local test target. The command did not reach the target test because the current checkout fails to compile first with unrelated proto initializer drift: missing middleware fields in NetworkPolicyRule / NetworkEndpoint initializers and missing network_middlewares fields in SandboxPolicy initializers.

Proposed Handling

Emit a warning from the proxy when an endpoint has an unsupported access value that cannot be expanded for its protocol. The warning should include enough safe context to diagnose the policy entry, such as policy name, endpoint host/port/protocol, and the unsupported access value.

This should stay proxy-side for now because the field is consumed by the proxy expansion path. The existing merge-layer UnsupportedAccessPreset check is useful precedent, but it does not replace a warning in openshell-supervisor-network.

Definition of Done

  • Unsupported L7 access values emit a warning instead of being silently ignored.
  • The warning is visible in normal proxy/sandbox logs.
  • The warning avoids logging secrets or query parameters.
  • A focused test covers an unsupported access value such as access: allow.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions