refactor(policy): extract shared L7 endpoint validation by gracesmith6504 · Pull Request #2389 · NVIDIA/OpenShell

gracesmith6504 · 2026-07-21T13:19:56Z

Summary

Extract 9 L7 endpoint semantic checks into a shared validate_l7_endpoint_semantics() function in openshell-policy. Both profile lint (openshell-providers) and the runtime validator (openshell-supervisor-network) now call the shared function via a lightweight L7EndpointFields field bag, eliminating duplication and preventing drift. Also consolidates the L7Protocol enum into a single definition in openshell-policy.

Related Issue

Fixes #1714

Changes

New shared validation module crates/openshell-policy/src/l7_validate.rs with L7Protocol enum, L7EndpointFields struct, validate_l7_endpoint_semantics() function, and 15 unit tests
Re-export shared validation types from crates/openshell-policy/src/lib.rs
Add openshell-policy dependency to crates/openshell-providers/Cargo.toml
Replace inline L7 checks with shared function call in crates/openshell-providers/src/profiles.rs; add 5 integration tests
Delete duplicate L7Protocol enum from crates/openshell-supervisor-network/src/l7/mod.rs, import and re-export from openshell-policy; replace 9 duplicated checks with shared function call; align rules_would_deny_all computation
Fix 4 test fixtures in crates/openshell-server/src/grpc/policy.rs and provider.rs: add access: "full" to endpoints with protocol

Testing

mise run pre-commit passes
Unit tests added/updated
E2E tests added/updated (if applicable)

Checklist

Follows Conventional Commits
Commits are signed off (DCO)
Architecture docs updated (if applicable)

copy-pr-bot · 2026-07-21T13:19:59Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

johntmyers

PR Review Status

Validation: This PR is project-valid because it implements the confirmed provider-v2 lint/runtime validation gap in #1714 using the shared policy-layer approach recommended during maintainer triage.
Head SHA: 73f81c6673a825dde5b41c8f1d8710b5cb89a1f9

Review findings:

Changes are requested for collection-presence regression, mixed-case MCP validation/runtime inconsistency, and divergent malformed-rule classification.
One additional suggestion asks the author to preserve or explicitly test the intended JSON-RPC diagnostic aggregation.

Docs: Not needed; this restores lint/runtime parity for already-documented endpoint semantics and does not introduce new user-facing behavior.

Next state: gator:in-review pending an author update.

gracesmith6504 · 2026-07-22T08:11:44Z

Addressed all comments:
added empty rules rejection, removed case normalization, fixed rules_would_deny_all for proto-converted data, and added exact error-list tests for the JSON-RPC diagnostic change. Ready for another look.

johntmyers

Re-check After Author Update

Thanks @gracesmith6504. I reviewed current head e26f89ffb1c7f09d3879c1a7422ab1f96b74e2d3 after your July 22 update. The case-sensitive protocol parsing and exact JSON-RPC diagnostic tests resolve those two prior concerns, and the runtime adapter now recognizes protobuf-converted empty allow objects.

Validation: This remains project-valid and implements the confirmed provider-v2 lint/runtime gap in #1714 using the shared policy-layer approach recommended during maintainer triage.
Head SHA: e26f89ffb1c7f09d3879c1a7422ab1f96b74e2d3

Review findings:

Two lint/runtime parity gaps remain in the provider-profile adapter: explicit rules: [] presence is lost during deserialization, and allow: {} is not classified as deny-all during lint.
Please add adapter-level parity coverage for these representations (and MCP allow-all) so the profile and runtime paths cannot drift while their field bags differ.

Docs: Not needed; this surfaces existing, already-documented endpoint constraints earlier and introduces no new user-facing schema or workflow.

Next state: gator:in-review pending an author update.

johntmyers

Re-check After Author Update

Thanks @gracesmith6504. I reviewed current head c422a8272dadb86024a795a9e1edc6f01e3b5041 after your July 22 update about empty-list handling, case-sensitive protocol parsing, proto-converted deny-all detection, and JSON-RPC diagnostics.

What I checked: the full current diff, both profile/runtime adapters, serde and protobuf conversions, MCP allow-all semantics, and the new regression coverage. The explicit rules: [] and REST allow: {} concerns are resolved.

Validation: This remains project-valid and implements the confirmed provider-v2 lint/runtime gap in #1714 using the shared policy-layer approach recommended during maintainer triage.
Head SHA: c422a8272dadb86024a795a9e1edc6f01e3b5041

Review findings:

One blocking MCP regression is anchored inline: selector-only MCP rules accepted under mcp.allow_all_known_mcp_methods: true are now misclassified as deny-all before alias normalization.
Please also add profile-adapter coverage for MCP allow-all with omitted rules and verify protobuf round-trip preservation; the shared validator unit test alone does not protect the adapter boundary.

Docs: Not needed; the intended MCP behavior is already documented, and this PR introduces no new user-facing schema or workflow.

Next state: gator:in-review pending an author update.

johntmyers

Re-check After Author Update

Thanks @gracesmith6504. I reviewed current head 88e367cf79e0c58045ef50ca33737c1a960cadd0 after your update addressing empty-list presence, MCP selector handling, and protobuf round-trip coverage.

What I checked: the complete eight-file diff, the profile/YAML → protobuf → runtime JSON → alias-normalization → Rego path, the prior review concerns, and adapter-boundary coverage. The explicit empty-list, case-sensitive protocol, JSON-RPC diagnostic, and selector-only MCP false-positive concerns are resolved.

Validation: This remains project-valid and implements the confirmed Provider V2 lint/runtime validation gap in #1714 using the shared policy-layer approach recommended during maintainer triage.
Head SHA: 88e367cf79e0c58045ef50ca33737c1a960cadd0

Review findings:

One high-severity MCP authorization regression remains: mixing a tool-specific rule with an empty allow rule under the MCP method profile can normalize to an implicit wildcard and bypass the tool restriction.
Two profile-adapter parity gaps remain for MCP deny-rule params round-tripping and unsupported params classification.

Docs: Not needed; this restores validation parity for an already documented policy schema and introduces no new workflow or navigation.

Next state: gator:in-review pending an author update.

gracesmith6504 · 2026-07-22T10:30:37Z

Addressed all three findings from the review on 88e367c:

MCP implicit wildcard (CWE-863): Added detection in both runtime (mod.rs) and
profile adapter (profiles.rs) for empty allow rules that normalize to method: "*"
when allow_all_known_mcp_methods is true, conflicting with tool-specific rules in
the same endpoint.
Deny-rule params round-trip: Added params: HashMap<String, L7QueryMatcherProfile>
to L7DenyRuleProfile with proto conversion in both directions. Round-trip test
added.
Unsupported params classification: Changed is_effectively_empty() to check
!self.params.contains_key("name") instead of self.params.is_empty(), matching the
runtime's mcp_rule_has_tool_selector which only recognizes params.name. Regression
test added for params: { arguments: ... } being classified as deny-all.

Ready for another look.

johntmyers

Re-check After Author Update

Thanks @gracesmith6504. I reviewed current head 4e2a61a79204f27dfad6cacf59182ea0e00b4890 after your July 22 update about the implicit MCP wildcard, deny-rule params round-tripping, and unsupported params classification.

What I checked: the complete current diff, Provider V2 YAML/profile conversions, shared lint/runtime validation, MCP alias normalization, Rego authorization behavior, and adapter-boundary tests. Your three stated fixes resolve the previously reported representations.

Validation: This remains project-valid and implements the confirmed Provider V2 lint/runtime gap in #1714 using the shared policy-layer approach recommended during maintainer triage.
Head SHA: 4e2a61a79204f27dfad6cacf59182ea0e00b4890

Review findings:

Two remaining profile-adapter parity defects are anchored inline. One can silently discard the documented MCP tool selector and broaden authorization; the other misses explicit broad MCP allow/deny conflicts that runtime validation rejects.
Please add adapter coverage for the tool alias, explicit broad allow/deny matchers, MCP allow-all with omitted rules, and exact matcher values across round trips.

Docs: No Fern update is needed for this refactor; the Provider V2 docs already promise the network-policy endpoint shape, so the tool behavior should be preserved or explicitly rejected in code.

Next state: gator:in-review pending an author update.

gracesmith6504 · 2026-07-22T13:01:04Z

Addressed both findings from the review on 4e2a61a:

Tool alias (CWE-863): Added tool field to both L7AllowProfile and
L7DenyRuleProfile. It gets lowered to params.name during proto conversion.
Validation rejects simultaneous tool + params.name. The has_tool_selector() helper
is used everywhere (is_effectively_empty, allow-rule conflict detection, deny-rule
conflict detection) so the field is consistently recognized across all paths.
Broad method parity: Added method_matcher_matches_tools_call() matching the
runtime predicate. Allow-rule conflict detection now catches explicit broad methods
(tools/call, , tools/) regardless of allow_all_known_mcp_methods. Added deny-rule
broad method conflict detection matching runtime behavior.

Ready for another look.

johntmyers

Re-check After Author Update

Thanks @gracesmith6504. I reviewed current head b1b8db51f764945091cba1dc3e2b0dfa044aec2f after your July 22 update about preserving the MCP tool alias and matching runtime broad-method behavior.

What I checked: the complete eight-file diff, provider-profile deserialization and protobuf lowering, shared lint/runtime validation, MCP alias normalization, OPA authorization, and the new adapter tests. The prior tool-discard and broad tools/call/*/tools/* parity findings are resolved for the object-matcher paths covered by this head.

Validation: This remains project-valid and implements the confirmed Provider V2 lint/runtime gap in #1714 using the shared policy-layer approach recommended during maintainer triage.
Head SHA: b1b8db51f764945091cba1dc3e2b0dfa044aec2f

Review findings:

Two profile-adapter defects are anchored inline: documented scalar MCP matchers do not deserialize, and deny-only dual selectors can bypass lint and silently discard tool during lowering.
One additional lint/runtime parity gap remains: MCP tool or params.name selectors are accepted without method: tools/call when allow-all is disabled, and with incompatible methods. Runtime rejects those shapes. Please mirror the runtime selector/method contract for both allow and deny rules and add missing/incompatible-method lint coverage.

Docs: No Fern update is needed for this refactor. Existing docs already describe provider endpoints as the sandbox policy shape and document scalar-or-object MCP matchers; the implementation should preserve that contract.

Next state: gator:in-review pending an author update.

gracesmith6504 · 2026-07-22T14:16:23Z

Addressed all three findings from the review on b1b8db5:

Scalar matcher deserialization: Added custom Deserialize impl for
L7QueryMatcherProfile that accepts both bare strings (tool: search_web) and object
form (tool: { glob: "read_*" }), matching the policy layer's QueryMatcherDef.
Guard placement: Moved mutual exclusivity check (tool + params.name) and
method/selector contract checks outside the has_tool_allow_selectors guard so they
fire for all MCP rules, including deny-only configurations.
Method/selector contract: Mirrored the runtime's full contract for both allow and
deny rules -- method must be tools/call when a tool selector is present, and method
is required when allow_all_known_mcp_methods is false.

5 new tests added (106 total), all passing.

Ready for another look.

johntmyers

Re-check After Author Update

Thanks @gracesmith6504. I reviewed current head cebeb9e855c78276a2aef18a25ce18cb85882695 after your July 22 update about preserving the MCP tool alias and matching broad-method behavior.

What I checked: the complete current diff, Provider V2 YAML/profile-to-protobuf import path, server-side lint, runtime validation, alias normalization, Rego authorization behavior, and the new adapter coverage. The scalar matcher and selector/method compatibility fixes resolve the corresponding prior findings.

Validation: This remains project-valid and implements the confirmed Provider V2 lint/runtime gap in #1714 using the shared policy-layer approach recommended during maintainer triage.
Head SHA: cebeb9e855c78276a2aef18a25ce18cb85882695

Review findings:

Three remaining lint/import parity defects are anchored inline.
The highest-severity issue lets protobuf lowering erase a conflicting MCP selector before server-side validation, changing the authored authorization intent.
The CLI-to-server protobuf boundary also loses empty-list presence, and profile lint still omits several MCP matcher checks enforced at runtime.

Docs: No Fern update is needed for the intended parity refactor; this should preserve the already documented policy contract.

Next state: gator:in-review pending an author update.

gracesmith6504 · 2026-07-22T15:07:13Z

Comment 16 – pre-lowering validation for tool/params.name

Added validate_before_lowering() on ProviderTypeProfile, called in the CLI's
load_profile_import_item before to_proto(). This catches the tool + params.name
mutual-exclusivity conflict before proto conversion erases the tool field. Three
regression tests cover allow rules, deny rules, and the clean path.

Comment 17 – empty list presence checks

rules: [] and deny_rules: [] collapse to None after proto round-trip, silently
bypassing the empty-list diagnostics in validate_profile_set. Fixed by adding both
checks to validate_before_lowering() (any protocol, not just MCP) so they fire
before proto conversion. Two regression tests.

Comment 18 – runtime MCP per-rule parity

Added the missing error-level checks from the runtime's per-rule MCP validation to
validate_profile_set, plus three helper functions (glob_uses_wildcard,
matcher_uses_glob_wildcard, validate_mcp_matcher). Every check applies to both allow
and deny rules:

Unsupported params keys (anything besides name)
Matcher glob + any mutual exclusion
Empty matcher (neither glob nor any specified)
Wildcard tool/params.name matchers require mcp.strict_tool_names to remain enabled
(defaults to true)
MCP method globs restricted to the tools/ family
path/query rejected on MCP rules
tool, params, and mcp options rejected on non-MCP endpoints

Six regression tests for the matcher checks plus one each for method globs, path
rejection, tool-on-non-MCP, and mcp-options-on-non-MCP.

120 tests passing, CI green.

johntmyers

Re-check After Author Update

Thanks @gracesmith6504. I reviewed current head b1d4deb2a3424c6c4a5e1ab2fdce9c81bfc06173 after your July 22 update about pre-lowering selector/list validation and the missing MCP per-rule checks.

What I checked: the complete current diff, CLI profile import/lint path, catalog/static-profile path, profile-to-protobuf conversion, shared and runtime validators, regression coverage, and the existing Fern policy/provider-profile documentation. Your three stated fixes resolve the prior findings.

Validation: This remains project-valid and implements the confirmed Provider V2 lint/runtime validation gap in #1714 using the shared policy-layer approach recommended during maintainer triage.
Head SHA: b1d4deb2a3424c6c4a5e1ab2fdce9c81bfc06173

Review findings:

One remaining validation-path defect is anchored inline: catalog/static profiles discard new L7 diagnostics that do not map to a legacy ProfileError.
The existing Fern docs also need correction: docs/reference/policy-schema.mdx should state the conditional non-empty rules/access requirement, and the example at docs/sandboxes/providers-v2.mdx:235 currently combines mutually exclusive access and rules. Navigation changes are not needed.
One inline suggestion preserves the structured endpoint location in CLI pre-lowering errors.

Docs: Required because lint-visible policy behavior is changing and the current provider-profile example is rejected by this contract.

E2E: No test:* label is needed; this does not change sandbox lifecycle, gateway/supervisor interaction, enforcement behavior, proxy behavior, credential flow, drivers, or runtime packaging.

Next state: gator:in-review pending an author update.

johntmyers

Re-check After Author Update

Thanks @gracesmith6504. I reviewed current head 1cac830a3598637db5251a0eac956cb8a26e4077 after your July 22 updates addressing the catalog/static-profile validation path, structured CLI diagnostics, and Fern documentation.

What I checked: the complete current diff; provider YAML deserialization and pre-lowering validation; protobuf, server, and catalog paths; runtime validation and alias normalization; authorization-sensitive matcher handling; regression coverage; and the corrected policy/provider-profile documentation. The previous catalog propagation, CLI diagnostic-location, conditional-requirement docs, and invalid provider example findings are resolved.

Validation: This remains project-valid and implements the confirmed Provider V2 lint/runtime validation gap in #1714 using the shared policy-layer approach recommended during maintainer triage.
Head SHA: 1cac830a3598637db5251a0eac956cb8a26e4077

Review findings:

Five remaining findings are anchored inline: one critical fail-open matcher-deserialization risk, three lint/runtime or normalization parity defects, and one documentation mismatch.
Author changes are required before pipeline monitoring.

Docs: The earlier example and conditional-requirement updates are present, but the new constraints still need the SQL and MCP deny-base exceptions corrected.

E2E: No test:* label is needed; this changes validation/lint paths and documentation without changing sandbox lifecycle, gateway/supervisor interaction, runtime policy enforcement behavior, proxy behavior, credential flow, drivers, or packaging.

Next state: gator:in-review pending an author update.

johntmyers

Re-check After Author Update

I reviewed current head 3be623ab1c8d16d70e9763318aeb9c97878ce063 against the complete diff and @gracesmith6504's earlier updates about matcher safety, MCP allow-all normalization, profile/runtime parity, and Fern documentation.

What I checked: Provider V2 YAML deserialization, pre-lowering validation, protobuf conversion, runtime validation and alias normalization, authorization-sensitive MCP rule handling, generic JSON-RPC/query matcher parity, regression coverage, and the updated policy-schema docs. The earlier SQL requirement and MCP deny-base documentation issues are resolved, and runtime enforcement remains fail-closed.

Validation: This remains project-valid and implements the confirmed Provider V2 lint/runtime validation gap in #1714 using the shared policy-layer direction recommended during maintainer triage.
Head SHA: 3be623ab1c8d16d70e9763318aeb9c97878ce063

Review findings:

Seven actionable findings are anchored inline: one input-shape safety issue, four lint/runtime or normalization parity defects, one protocol parsing compatibility regression, and one Fern documentation omission.
I found no direct authorization fail-open, but author changes and focused regression tests are required before pipeline monitoring.

Docs: The SQL and MCP exception text is now accurate, but the newly enforced empty deny_rules constraint is not documented. Navigation changes are not needed.

E2E: Deferred until review findings are resolved; no test label is being applied in this cycle.

Next state: gator:in-review pending an author update.

gracesmith6504 · 2026-07-23T09:40:48Z

Hi @johntmyers, thanks for the thorough reviews. I've learned a lot
working through these rounds.

I wanted to ask about scope. The original issue (#1714)
was extracting the 9 shared L7 endpoint semantic checks
to prevent drift between profile lint and runtime. That
part is working and covered by tests.

The later rounds have expanded into full MCP
matcher/tool/params parity, generic query matcher
validation, and JSON-RPC per-rule checks. Those are
important, but they were pre-existing gaps outside the
scope of my change.

Would it make sense to land the core shared validation in
this PR and open a follow-up for the broader matcher
parity work? Happy to do it either way, just wanted to
check before the PR grows further.

johntmyers · 2026-07-23T12:34:14Z

/ok to test 3be623a

johntmyers · 2026-07-23T16:01:02Z

Yes I think that makes perfect sense.

johntmyers · 2026-07-23T17:12:49Z

@gracesmith6504 I'm trying out a little guardrail with the gator agent to see if it can stop scope creeping but will try and get this merged today. do you have anything else you're planning to commit?

johntmyers

Scope-Focused Re-review

Thanks @gracesmith6504 for calling out the scope expansion. @johntmyers agreed that full MCP matcher tool/params parity, generic query matcher validation, JSON-RPC per-rule checks, and the other pre-existing broad parity gaps belong in follow-up work. I re-reviewed the complete current diff on that reduced basis.

Validation: This PR remains project-valid and addresses #1714 by sharing the listed L7 endpoint semantic checks between Provider v2 profile lint and runtime validation.
Head SHA: 3be623ab1c8d16d70e9763318aeb9c97878ce063

Review findings:

The broad matcher-parity items from the prior review are non-blocking follow-ups for this PR.
Two PR-introduced, original-scope issues remain and are anchored inline: the shared protocol parser breaks the runtime's prior case-insensitive behavior, and the newly lint-visible rejection of non-empty rule lists with no effective allow clause is undocumented.
No authorization fail-open or sandbox-integrity regression was found; validation remains fail-closed.

Docs: Navigation changes are unnecessary, but the new deny-all rule-list validation needs the focused policy-schema clarification noted inline.

E2E: test:e2e is required because this changes policy enforcement and network-proxy validation, but it is deferred until these review findings are resolved and a new head is available.

Next state: gator:in-review pending an author update.

johntmyers

Scope-Focused Re-review

Thanks @gracesmith6504 for identifying the scope expansion. @johntmyers agreed that full MCP matcher tool/params parity, generic query matcher validation, JSON-RPC per-rule checks, and the other pre-existing broad parity gaps belong in follow-up work. I independently re-reviewed the complete current diff against only the nine shared L7 endpoint semantic checks from #1714.

Validation: This PR remains project-valid and the shared-validator approach is sound.
Head SHA: 3be623ab1c8d16d70e9763318aeb9c97878ce063

Review findings:

Two PR-introduced original-scope compatibility regressions are anchored inline: protocol parsing no longer preserves the runtime's prior case-insensitive behavior, and MCP allow-all with an empty allow object is incorrectly classified as deny-all before normalization.
The earlier broad matcher-parity findings are non-blocking follow-up work for this PR.
No authorization fail-open or sandbox-integrity regression was found; default-deny behavior remains intact.

Docs: One focused clarification is suggested inline for the newly shared rules-would-deny-all constraint; no navigation change is needed.

E2E: test:e2e will be required after the blocking review findings are resolved because this changes policy enforcement and network-proxy validation. It is not being applied on this head.

Next state: gator:in-review pending an author update.

Move L7 endpoint semantic checks into the openshell-policy crate so both profile lint and the runtime validator share one implementation. This eliminates drift between the two validation paths. The shared validator covers 9 checks: unknown protocol, rules/access mutual exclusivity, JSON-RPC family access rejection, json-rpc requires rules, non-JSON-RPC protocol requires rules or access, MCP requires rules when allow_all is false, rules-would-deny-all detection, deny_rules require protocol, and deny_rules require base allow set. Changes rules/deny_rules fields to Option<Vec<...>> so absent vs empty is distinguishable at lint time. Adds is_effectively_empty() to L7AllowProfile for deny-all detection of allow: {} objects. Makes rules_would_deny_all MCP-aware by checking tool/params.name selectors before classifying a rule as deny-all. Adds params field to L7AllowProfile so MCP tool selectors survive proto round-trip. Signed-off-by: Grace Smith <gsmith@redhat.com> Signed-off-by: Grace Smith <grasmith@redhat.com>

gracesmith6504 · 2026-07-24T10:23:36Z

Hi @johntmyers thank you for your help, this should be the last update (8bdd502). Addressed both findings from the
scoped re-review:

Restored case-insensitive protocol parsing in L7Protocol::parse (added
to_ascii_lowercase()). Updated tests to cover mixed-case inputs like REST and Mcp.
Guarded the deny-all check for MCP with allow_all_known_mcp_methods: true, since runtime
normalization supplies method: "*" for empty allows. Added regression tests in both the
profile lint and runtime validation paths.

Also added the docs clarification about rules needing at least one effective allow clause.

Nothing else planned to commit.

github-actions · 2026-07-24T10:43:01Z

Label test:e2e applied, but pull-request/2389 is at 3be623a while the PR head is 8bdd502. A maintainer needs to comment /ok to test 8bdd50298570aa16c115eab0ec4605b86994f74e to refresh the mirror. Once the mirror catches up, re-run Branch E2E Checks from the Actions tab.

johntmyers · 2026-07-24T10:43:10Z

/ok to test 8bdd502

johntmyers

Re-check After Author Update

Thanks @gracesmith6504. I reviewed current head 8bdd50298570aa16c115eab0ec4605b86994f74e after your final update addressing the two scope-focused findings. @johntmyers confirmed that the broader pre-existing matcher-parity work belongs in follow-up work, so I kept this review focused on the shared L7 endpoint semantics in #1714 while still checking the complete current diff for regressions and security issues.

What I checked: the full 11-file diff; both profile-lint and runtime adapters; protocol parsing; MCP allow-all normalization and deny-all classification; protobuf/profile conversion paths; regression coverage; and the focused Fern policy reference update.

Validation: This PR remains project-valid and implements the confirmed Provider V2 lint/runtime gap in #1714 using the shared policy-layer approach recommended during maintainer triage.
Head SHA: 8bdd50298570aa16c115eab0ec4605b86994f74e

Review findings:

The case-insensitive protocol compatibility regression is resolved.
The MCP allow-all empty-rule false positive is resolved in both callers with focused coverage.
No blocking correctness, security, maintainability, test, or documentation findings remain.

Docs: The policy reference now documents the effective-allow-clause constraint; navigation changes are not needed.

E2E: test:e2e is applied because this changes policy enforcement and network-proxy validation. /ok to test 8bdd50298570aa16c115eab0ec4605b86994f74e has been posted, and required checks are pending.

Next state: gator:watch-pipeline.

johntmyers · 2026-07-24T18:07:01Z

Monitoring Complete

Monitoring is complete because this PR has merged.

Final status: head 8bdd50298570aa16c115eab0ec4605b86994f74e had no blocking review findings, required E2E coverage completed, required checks were green, and maintainer approval was present before merge.

I removed the active gator:* label because there is nothing left for gator to monitor on this PR.

Resolve conflicts with the corporate HTTP proxy work (NVIDIA#2245), the CLI commands/common extraction (NVIDIA#2359), the supervisor middleware runtime (NVIDIA#2027), and the shared L7 endpoint validation refactor (NVIDIA#2389): - openshell-core/src/net.rs: keep both import sets — main's `ipnet` CIDR types alongside `SocketAddr`/`TcpStream` for the TCP_NODELAY helpers. - openshell-cli/src/run.rs: main moved the `progress` constants into the test module; keep only the `net` import at the top level. - openshell-server/src/lib.rs: union of both import lines. - supervisor-network/src/proxy.rs: main replaced the CONNECT tunnel's direct dial with proxy-aware `dial_upstream`. Take main's call and push TCP_NODELAY down into `dial_upstream`'s direct paths plus `upstream_proxy::connect_via_inner`, so tunnel hops keep it whether or not they chain through a corporate proxy. The plain-HTTP forward path keeps main's direct-dial comment with the NODELAY-setting connect. Signed-off-by: Jim Meyer <jim@meyer4hire.com>

gracesmith6504 requested review from a team, derekwaynecarr, maxamillion and mrunalp as code owners July 21, 2026 13:19

johntmyers reviewed Jul 21, 2026

View reviewed changes

Comment thread crates/openshell-policy/src/l7_validate.rs

Comment thread crates/openshell-policy/src/l7_validate.rs

Comment thread crates/openshell-supervisor-network/src/l7/mod.rs Outdated

Comment thread crates/openshell-policy/src/l7_validate.rs

johntmyers added the gator:in-review Gator is reviewing or awaiting PR review feedback label Jul 21, 2026

gracesmith6504 force-pushed the fix/1714-shared-l7-validation branch from 73f81c6 to e26f89f Compare July 22, 2026 08:09

johntmyers reviewed Jul 22, 2026

View reviewed changes

Comment thread crates/openshell-providers/src/profiles.rs Outdated

Comment thread crates/openshell-providers/src/profiles.rs Outdated

gracesmith6504 force-pushed the fix/1714-shared-l7-validation branch from e26f89f to c422a82 Compare July 22, 2026 08:44

johntmyers reviewed Jul 22, 2026

View reviewed changes

Comment thread crates/openshell-supervisor-network/src/l7/mod.rs

gracesmith6504 force-pushed the fix/1714-shared-l7-validation branch from c422a82 to 88e367c Compare July 22, 2026 09:49

johntmyers reviewed Jul 22, 2026

View reviewed changes

Comment thread crates/openshell-supervisor-network/src/l7/mod.rs

Comment thread crates/openshell-providers/src/profiles.rs

Comment thread crates/openshell-providers/src/profiles.rs Outdated

gracesmith6504 force-pushed the fix/1714-shared-l7-validation branch from 88e367c to 4e2a61a Compare July 22, 2026 10:18

johntmyers reviewed Jul 22, 2026

View reviewed changes

Comment thread crates/openshell-providers/src/profiles.rs

Comment thread crates/openshell-providers/src/profiles.rs Outdated

gracesmith6504 force-pushed the fix/1714-shared-l7-validation branch from 4e2a61a to b1b8db5 Compare July 22, 2026 12:53

johntmyers reviewed Jul 22, 2026

View reviewed changes

Comment thread crates/openshell-providers/src/profiles.rs

Comment thread crates/openshell-providers/src/profiles.rs

gracesmith6504 force-pushed the fix/1714-shared-l7-validation branch from b1b8db5 to cebeb9e Compare July 22, 2026 13:58

johntmyers reviewed Jul 22, 2026

View reviewed changes

Comment thread crates/openshell-providers/src/profiles.rs

Comment thread crates/openshell-providers/src/profiles.rs

Comment thread crates/openshell-providers/src/profiles.rs

gracesmith6504 force-pushed the fix/1714-shared-l7-validation branch from cebeb9e to b1d4deb Compare July 22, 2026 15:05

johntmyers reviewed Jul 22, 2026

View reviewed changes

Comment thread crates/openshell-providers/src/profiles.rs

Comment thread crates/openshell-cli/src/run.rs Outdated

gracesmith6504 force-pushed the fix/1714-shared-l7-validation branch from b1d4deb to 1cac830 Compare July 22, 2026 16:08

johntmyers reviewed Jul 22, 2026

View reviewed changes

Comment thread crates/openshell-providers/src/profiles.rs

Comment thread crates/openshell-supervisor-network/src/l7/mod.rs

Comment thread crates/openshell-providers/src/profiles.rs

Comment thread crates/openshell-providers/src/profiles.rs

Comment thread docs/reference/policy-schema.mdx Outdated

gracesmith6504 force-pushed the fix/1714-shared-l7-validation branch from 1cac830 to 3be623a Compare July 23, 2026 07:19

johntmyers reviewed Jul 23, 2026

View reviewed changes

Comment thread crates/openshell-policy/src/l7_validate.rs Outdated

Comment thread docs/reference/policy-schema.mdx

johntmyers reviewed Jul 23, 2026

View reviewed changes

Comment thread crates/openshell-policy/src/l7_validate.rs Outdated

Comment thread crates/openshell-policy/src/l7_validate.rs Outdated

Comment thread docs/reference/policy-schema.mdx

gracesmith6504 force-pushed the fix/1714-shared-l7-validation branch from 3be623a to 8bdd502 Compare July 24, 2026 10:22

johntmyers added the test:e2e Requires end-to-end coverage label Jul 24, 2026

johntmyers reviewed Jul 24, 2026

View reviewed changes

johntmyers approved these changes Jul 24, 2026

View reviewed changes

johntmyers added this pull request to the merge queue Jul 24, 2026

johntmyers added gator:merge-ready and removed gator:approval-needed Gator completed review; maintainer approval needed labels Jul 24, 2026

Merged via the queue into NVIDIA:main with commit deced87 Jul 24, 2026
85 of 91 checks passed

johntmyers removed the gator:merge-ready label Jul 24, 2026

Uh oh!

Conversation

gracesmith6504 commented Jul 21, 2026

Summary

Related Issue

Changes

Testing

Checklist

Uh oh!

copy-pr-bot Bot commented Jul 21, 2026

Uh oh!

johntmyers left a comment

Choose a reason for hiding this comment

PR Review Status

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gracesmith6504 commented Jul 22, 2026

Uh oh!

johntmyers left a comment

Choose a reason for hiding this comment

Re-check After Author Update

Uh oh!

Uh oh!

Uh oh!

johntmyers left a comment

Choose a reason for hiding this comment

Re-check After Author Update

Uh oh!

Uh oh!

johntmyers left a comment

Choose a reason for hiding this comment

Re-check After Author Update

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gracesmith6504 commented Jul 22, 2026

Uh oh!

johntmyers left a comment

Choose a reason for hiding this comment

Re-check After Author Update

Uh oh!

Uh oh!

Uh oh!

gracesmith6504 commented Jul 22, 2026

Uh oh!

johntmyers left a comment

Choose a reason for hiding this comment

Re-check After Author Update

Uh oh!

Uh oh!

Uh oh!

gracesmith6504 commented Jul 22, 2026

Uh oh!

johntmyers left a comment

Choose a reason for hiding this comment

Re-check After Author Update

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gracesmith6504 commented Jul 22, 2026

Uh oh!

johntmyers left a comment

Choose a reason for hiding this comment

Re-check After Author Update

Uh oh!

Uh oh!

Uh oh!

johntmyers left a comment

Choose a reason for hiding this comment

Re-check After Author Update

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!