Skip to content

Commit 2240928

Browse files
Pangjipingclaude
andauthored
feat(egress): add DELETE /policy endpoint for removing egress rules by target (#864)
* feat(egress): add DELETE /policy endpoint for removing egress rules by target Add DELETE handler that accepts a JSON array of target strings, removes matching rules case-insensitively, and commits the updated policy. Targets not found are silently ignored (idempotent). Spec and README docs updated. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(sdks): expose egress DELETE /policy across all sandbox SDKs Wires the new DELETE /policy endpoint through Go, JavaScript, Python (async + sync), Kotlin, and C# sandbox SDKs so users can remove egress rules by target through the supported facades. Regenerates the JS and Python OpenAPI clients (TypeScript and Kotlin generators now emit the delete operation; Python generator produces a new delete_policy module), then adds matching handwritten adapter/service/sandbox methods and unit tests. Extends the C# HttpClientWrapper with a DeleteAsync(path, body, ct) overload since DELETE-with-body was not previously supported. Adds an async Python e2e test (test_01ac_network_policy_delete) that provisions a sandbox with two allow rules, deletes one (plus a nonexistent target to verify idempotency), and confirms the policy mutation, defaultAction preservation, and resulting traffic behavior. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(egress): DELETE response shape + status codes - Drop Policy field from no-match path (consistent with POST/PATCH success responses and the spec's DELETE example). - Return 500 instead of 400 for marshal/parse failures on self-synthesized policy JSON (internal inconsistency, not client error); upgrade matching log level to Error. - Revert unrelated trailing-whitespace cleanup in smoke-nft.sh copyright header. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 637b981 commit 2240928

31 files changed

Lines changed: 914 additions & 8 deletions

File tree

components/egress/policy_server.go

Lines changed: 88 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,10 @@ func (s *policyServer) handlePolicy(w http.ResponseWriter, r *http.Request) {
147147
s.handlePost(w, r)
148148
case http.MethodPatch:
149149
s.handlePatch(w, r)
150+
case http.MethodDelete:
151+
s.handleDelete(w, r)
150152
default:
151-
w.Header().Set("Allow", "GET, POST, PUT, PATCH")
153+
w.Header().Set("Allow", "GET, POST, PUT, PATCH, DELETE")
152154
http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
153155
}
154156
}
@@ -222,15 +224,16 @@ func (s *policyServer) handlePatch(w http.ResponseWriter, r *http.Request) {
222224
defer s.mu.Unlock()
223225

224226
raw, err := readPolicyRequestBody(r)
225-
if err != nil || raw == "" {
226-
if err != nil {
227-
logEgressUpdateFailedWarn(fmt.Sprintf("failed to read body: %v", err))
228-
} else {
229-
logEgressUpdateFailedWarn("empty patch body")
230-
}
227+
if err != nil {
228+
logEgressUpdateFailedWarn(fmt.Sprintf("failed to read body: %v", err))
231229
http.Error(w, fmt.Sprintf("failed to read body: %v", err), http.StatusBadRequest)
232230
return
233231
}
232+
if raw == "" {
233+
logEgressUpdateFailedWarn("empty patch body")
234+
http.Error(w, "empty body", http.StatusBadRequest)
235+
return
236+
}
234237

235238
var patchRules []policy.EgressRule
236239
if err := json.Unmarshal([]byte(raw), &patchRules); err != nil {
@@ -268,6 +271,84 @@ func (s *policyServer) handlePatch(w http.ResponseWriter, r *http.Request) {
268271
})
269272
}
270273

274+
func (s *policyServer) handleDelete(w http.ResponseWriter, r *http.Request) {
275+
defer r.Body.Close()
276+
s.mu.Lock()
277+
defer s.mu.Unlock()
278+
279+
raw, err := readPolicyRequestBody(r)
280+
if err != nil {
281+
logEgressUpdateFailedWarn(fmt.Sprintf("failed to read body: %v", err))
282+
http.Error(w, fmt.Sprintf("failed to read body: %v", err), http.StatusBadRequest)
283+
return
284+
}
285+
if raw == "" {
286+
logEgressUpdateFailedWarn("empty delete body")
287+
http.Error(w, "empty body", http.StatusBadRequest)
288+
return
289+
}
290+
291+
var targets []string
292+
if err := json.Unmarshal([]byte(raw), &targets); err != nil {
293+
logEgressUpdateFailedWarn(fmt.Sprintf("invalid delete targets: %v", err))
294+
http.Error(w, fmt.Sprintf("invalid delete targets: %v", err), http.StatusBadRequest)
295+
return
296+
}
297+
if len(targets) == 0 {
298+
logEgressUpdateFailedWarn("empty delete targets array")
299+
http.Error(w, "invalid delete targets: empty array", http.StatusBadRequest)
300+
return
301+
}
302+
303+
base := s.proxy.CurrentPolicy()
304+
if base == nil {
305+
base = policy.DefaultDenyPolicy()
306+
}
307+
oldCount := len(base.Egress)
308+
newEgress, removedRules := removeRulesByTarget(base.Egress, targets)
309+
removed := oldCount - len(newEgress)
310+
311+
if removed == 0 {
312+
mode := modeFromPolicy(base)
313+
writeJSON(w, http.StatusOK, policyStatusResponse{
314+
Status: "ok",
315+
Mode: mode,
316+
EnforcementMode: s.enforcementMode,
317+
Reason: "no matching targets found",
318+
})
319+
return
320+
}
321+
322+
rawMerged, err := json.Marshal(policy.NetworkPolicy{
323+
DefaultAction: base.DefaultAction,
324+
Egress: newEgress,
325+
})
326+
if err != nil {
327+
logEgressUpdateFailedError(fmt.Sprintf("failed to marshal updated policy: %v", err))
328+
http.Error(w, fmt.Sprintf("internal error: %v", err), http.StatusInternalServerError)
329+
return
330+
}
331+
newPolicy, err := policy.ParsePolicy(string(rawMerged))
332+
if err != nil {
333+
logEgressUpdateFailedError(fmt.Sprintf("invalid policy after delete: %v", err))
334+
http.Error(w, fmt.Sprintf("internal error: %v", err), http.StatusInternalServerError)
335+
return
336+
}
337+
338+
mode := modeFromPolicy(newPolicy)
339+
log.Infof("policy API: deleting %d egress rule(s) by target, removed=%d, mode=%s, enforcement=%s", len(targets), removed, mode, s.enforcementMode)
340+
if !s.commitPolicy(r.Context(), w, newPolicy, "delete") {
341+
return
342+
}
343+
logEgressUpdated(newPolicy.DefaultAction, removedRules)
344+
log.Infof("policy API: delete applied successfully")
345+
writeJSON(w, http.StatusOK, policyStatusResponse{
346+
Status: "ok",
347+
Mode: mode,
348+
EnforcementMode: s.enforcementMode,
349+
})
350+
}
351+
271352
// commitPolicy applies one logical change: optional disk persist → merge always file rules → nft
272353
// static (with nameserver allow-IPs) → then update in-memory user policy (POST/PATCH/GET view).
273354
func (s *policyServer) commitPolicy(ctx context.Context, w http.ResponseWriter, pol *policy.NetworkPolicy, op string) bool {

components/egress/policy_server_test.go

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,150 @@ func TestHandlePatch_RejectsWhenOverMaxEgressRules(t *testing.T) {
245245
require.Len(t, proxy.updated.Egress, 2, "policy should be unchanged")
246246
}
247247

248+
func TestHandleDelete_RemovesMatchingTargets(t *testing.T) {
249+
initial := &policy.NetworkPolicy{
250+
DefaultAction: policy.ActionDeny,
251+
Egress: []policy.EgressRule{
252+
{Action: policy.ActionAllow, Target: "example.com"},
253+
{Action: policy.ActionDeny, Target: "blocked.com"},
254+
{Action: policy.ActionAllow, Target: "keep.com"},
255+
},
256+
}
257+
proxy := &stubProxy{updated: initial}
258+
nft := &stubNft{}
259+
srv := &policyServer{proxy: proxy, nft: nft, enforcementMode: "dns+nft"}
260+
261+
body := `["blocked.com","nonexistent.com"]`
262+
req := httptest.NewRequest(http.MethodDelete, "/policy", strings.NewReader(body))
263+
w := httptest.NewRecorder()
264+
265+
srv.handlePolicy(w, req)
266+
267+
resp := w.Result()
268+
require.Equal(t, http.StatusOK, resp.StatusCode, "expected 200 OK")
269+
require.Equal(t, 1, nft.calls, "expected nft ApplyStatic called once")
270+
require.NotNil(t, proxy.updated, "expected proxy policy updated")
271+
require.Equal(t, policy.ActionDeny, proxy.updated.DefaultAction, "defaultAction should be preserved")
272+
require.Len(t, proxy.updated.Egress, 2, "expected 2 rules remaining after delete")
273+
require.Equal(t, policy.ActionAllow, proxy.updated.Egress[0].Action)
274+
require.Equal(t, "example.com", proxy.updated.Egress[0].Target)
275+
require.Equal(t, policy.ActionAllow, proxy.updated.Egress[1].Action)
276+
require.Equal(t, "keep.com", proxy.updated.Egress[1].Target)
277+
}
278+
279+
func TestHandleDelete_CaseInsensitiveMatch(t *testing.T) {
280+
initial := &policy.NetworkPolicy{
281+
DefaultAction: policy.ActionDeny,
282+
Egress: []policy.EgressRule{
283+
{Action: policy.ActionAllow, Target: "Example.COM"},
284+
{Action: policy.ActionDeny, Target: "Blocked.COM"},
285+
},
286+
}
287+
proxy := &stubProxy{updated: initial}
288+
nft := &stubNft{}
289+
srv := &policyServer{proxy: proxy, nft: nft, enforcementMode: "dns+nft"}
290+
291+
body := `["example.com"]`
292+
req := httptest.NewRequest(http.MethodDelete, "/policy", strings.NewReader(body))
293+
w := httptest.NewRecorder()
294+
295+
srv.handlePolicy(w, req)
296+
297+
resp := w.Result()
298+
require.Equal(t, http.StatusOK, resp.StatusCode, "expected 200 OK")
299+
require.NotNil(t, proxy.updated)
300+
require.Len(t, proxy.updated.Egress, 1, "expected 1 rule remaining")
301+
require.Equal(t, "Blocked.COM", proxy.updated.Egress[0].Target, "unmatched rule should remain")
302+
}
303+
304+
func TestHandleDelete_NoMatchReturns200(t *testing.T) {
305+
initial := &policy.NetworkPolicy{
306+
DefaultAction: policy.ActionDeny,
307+
Egress: []policy.EgressRule{
308+
{Action: policy.ActionAllow, Target: "keep.com"},
309+
},
310+
}
311+
proxy := &stubProxy{updated: initial}
312+
nft := &stubNft{}
313+
srv := &policyServer{proxy: proxy, nft: nft, enforcementMode: "dns+nft"}
314+
315+
body := `["nonexistent.com"]`
316+
req := httptest.NewRequest(http.MethodDelete, "/policy", strings.NewReader(body))
317+
w := httptest.NewRecorder()
318+
319+
srv.handlePolicy(w, req)
320+
321+
resp := w.Result()
322+
require.Equal(t, http.StatusOK, resp.StatusCode, "expected 200 OK even when no targets match")
323+
require.Equal(t, 0, nft.calls, "nft should not be called when nothing changes")
324+
require.Len(t, proxy.updated.Egress, 1, "policy should be unchanged")
325+
}
326+
327+
func TestHandleDelete_EmptyBodyReturns400(t *testing.T) {
328+
proxy := &stubProxy{updated: policy.DefaultDenyPolicy()}
329+
srv := &policyServer{proxy: proxy, nft: nil, enforcementMode: "dns"}
330+
331+
req := httptest.NewRequest(http.MethodDelete, "/policy", strings.NewReader(""))
332+
w := httptest.NewRecorder()
333+
334+
srv.handlePolicy(w, req)
335+
336+
resp := w.Result()
337+
require.Equal(t, http.StatusBadRequest, resp.StatusCode, "expected 400 for empty body")
338+
}
339+
340+
func TestHandleDelete_EmptyArrayReturns400(t *testing.T) {
341+
proxy := &stubProxy{updated: policy.DefaultDenyPolicy()}
342+
srv := &policyServer{proxy: proxy, nft: nil, enforcementMode: "dns"}
343+
344+
body := `[]`
345+
req := httptest.NewRequest(http.MethodDelete, "/policy", strings.NewReader(body))
346+
w := httptest.NewRecorder()
347+
348+
srv.handlePolicy(w, req)
349+
350+
resp := w.Result()
351+
require.Equal(t, http.StatusBadRequest, resp.StatusCode, "expected 400 for empty array")
352+
}
353+
354+
func TestHandleDelete_InvalidJSONReturns400(t *testing.T) {
355+
proxy := &stubProxy{updated: policy.DefaultDenyPolicy()}
356+
srv := &policyServer{proxy: proxy, nft: nil, enforcementMode: "dns"}
357+
358+
body := `not-json`
359+
req := httptest.NewRequest(http.MethodDelete, "/policy", strings.NewReader(body))
360+
w := httptest.NewRecorder()
361+
362+
srv.handlePolicy(w, req)
363+
364+
resp := w.Result()
365+
require.Equal(t, http.StatusBadRequest, resp.StatusCode, "expected 400 for invalid JSON")
366+
}
367+
368+
func TestHandleDelete_NftFailureReturns500(t *testing.T) {
369+
initial := &policy.NetworkPolicy{
370+
DefaultAction: policy.ActionDeny,
371+
Egress: []policy.EgressRule{
372+
{Action: policy.ActionAllow, Target: "example.com"},
373+
},
374+
}
375+
proxy := &stubProxy{updated: initial}
376+
nft := &stubNft{err: errors.New("nft apply failed")}
377+
srv := &policyServer{proxy: proxy, nft: nft, enforcementMode: "dns+nft"}
378+
379+
body := `["example.com"]`
380+
req := httptest.NewRequest(http.MethodDelete, "/policy", strings.NewReader(body))
381+
w := httptest.NewRecorder()
382+
383+
srv.handlePolicy(w, req)
384+
385+
resp := w.Result()
386+
require.Equal(t, http.StatusInternalServerError, resp.StatusCode, "expected 500 on nft failure")
387+
require.Equal(t, 1, nft.calls, "expected nft ApplyStatic called once")
388+
require.Len(t, proxy.updated.Egress, 1, "proxy should not be updated on nft failure")
389+
require.Equal(t, "example.com", proxy.updated.Egress[0].Target, "original rule should remain")
390+
}
391+
248392
func TestHandlePost_RejectsWhenOverMaxEgressRules(t *testing.T) {
249393
proxy := &stubProxy{}
250394
nft := &stubNft{}

components/egress/policy_utils.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,32 @@ func mergeEgressRules(base, additions []policy.EgressRule) []policy.EgressRule {
8383
return out
8484
}
8585

86+
// removeRulesByTarget returns a new slice with rules matching targets removed,
87+
// plus the removed rules. Domain targets are matched case-insensitively.
88+
// Targets not found are silently ignored.
89+
func removeRulesByTarget(rules []policy.EgressRule, targets []string) (kept, removed []policy.EgressRule) {
90+
if len(targets) == 0 || len(rules) == 0 {
91+
return rules, nil
92+
}
93+
removeSet := make(map[string]struct{}, len(targets))
94+
for _, t := range targets {
95+
key := strings.ToLower(strings.TrimSpace(t))
96+
if key == "" {
97+
continue
98+
}
99+
removeSet[key] = struct{}{}
100+
}
101+
kept = make([]policy.EgressRule, 0, len(rules))
102+
for _, r := range rules {
103+
if _, ok := removeSet[strings.ToLower(r.Target)]; ok {
104+
removed = append(removed, r)
105+
} else {
106+
kept = append(kept, r)
107+
}
108+
}
109+
return kept, removed
110+
}
111+
86112
// mergeKey: domain targets lowercased for dedupe; IP/CIDR left as-is.
87113
func mergeKey(r policy.EgressRule) string {
88114
if r.Target == "" {

components/egress/tests/smoke-nft.sh

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,47 @@ else
155155
pass "www.mozilla.org blocked after patch"
156156
fi
157157

158+
info "DELETE: deny two hosts, then delete one rule"
159+
curl -sSf -XPOST "http://127.0.0.1:${POLICY_PORT}/policy" \
160+
-d '{"defaultAction":"allow","egress":[{"action":"deny","target":"api.github.com"},{"action":"deny","target":"www.cloudflare.com"}]}'
161+
162+
info "Test: both hosts should be blocked before delete"
163+
if run_in_app -I https://api.github.com --max-time 8 >/dev/null 2>&1; then
164+
fail "api.github.com should be blocked before delete"
165+
fi
166+
if run_in_app -I https://www.cloudflare.com --max-time 8 >/dev/null 2>&1; then
167+
fail "www.cloudflare.com should be blocked before delete"
168+
fi
169+
pass "both hosts blocked before delete"
170+
171+
info "Deleting api.github.com rule"
172+
curl -sSf -XDELETE "http://127.0.0.1:${POLICY_PORT}/policy" \
173+
-d '["api.github.com"]'
174+
175+
info "Test: api.github.com allowed, www.cloudflare.com still blocked after delete"
176+
run_in_app -I https://api.github.com --max-time 20 >/dev/null 2>&1 || fail "api.github.com should be allowed after delete"
177+
pass "api.github.com allowed after delete"
178+
if run_in_app -I https://www.cloudflare.com --max-time 8 >/dev/null 2>&1; then
179+
fail "www.cloudflare.com should remain blocked after delete"
180+
fi
181+
pass "www.cloudflare.com still blocked"
182+
183+
info "Deleting non-existent target (idempotent)"
184+
resp="$(curl -sSf -XDELETE "http://127.0.0.1:${POLICY_PORT}/policy" -d '["nonexistent.com"]')"
185+
if echo "${resp}" | grep -q '"no matching targets found"'; then
186+
pass "idempotent delete returns no matching targets found"
187+
else
188+
fail "expected no matching targets found, got: ${resp}"
189+
fi
190+
191+
info "Deleting with empty body (expect 400)"
192+
http_code="$(curl -s -o /dev/null -w '%{http_code}' -XDELETE "http://127.0.0.1:${POLICY_PORT}/policy" -d '')"
193+
if [ "${http_code}" = "400" ]; then
194+
pass "empty body returns 400"
195+
else
196+
fail "empty body should return 400, got ${http_code}"
197+
fi
198+
158199
info "Always-rule dynamic check (single transition)"
159200
curl -sSf -XPOST "http://127.0.0.1:${POLICY_PORT}/policy" \
160201
-d '{"defaultAction":"deny","egress":[{"action":"allow","target":"api.github.com"}]}'

sdks/sandbox/csharp/src/OpenSandbox/Adapters/EgressAdapter.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,13 @@ public async Task PatchRulesAsync(
5454
await _client.PatchAsync("/policy", normalizedRules, cancellationToken).ConfigureAwait(false);
5555
}
5656

57+
public async Task DeleteRulesAsync(
58+
IReadOnlyList<string> targets,
59+
CancellationToken cancellationToken = default)
60+
{
61+
await _client.DeleteAsync("/policy", targets.ToList(), cancellationToken).ConfigureAwait(false);
62+
}
63+
5764
private static NetworkPolicy ParseNetworkPolicy(JsonElement element)
5865
{
5966
var policy = new NetworkPolicy();

sdks/sandbox/csharp/src/OpenSandbox/Internal/HttpClientWrapper.cs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,23 @@ public async Task DeleteAsync(
189189
await EnsureSuccessAsync(response, cancellationToken).ConfigureAwait(false);
190190
}
191191

192+
public async Task DeleteAsync(
193+
string path,
194+
object body,
195+
CancellationToken cancellationToken)
196+
{
197+
var url = BuildUrl(path);
198+
_logger.LogDebug("HTTP DELETE {Url}", url);
199+
using var request = new HttpRequestMessage(HttpMethod.Delete, url);
200+
ApplyDefaultHeaders(request);
201+
202+
var json = JsonSerializer.Serialize(body, JsonOptions);
203+
request.Content = new StringContent(json, Encoding.UTF8, "application/json");
204+
205+
using var response = await _httpClient.SendAsync(request, cancellationToken).ConfigureAwait(false);
206+
await EnsureSuccessAsync(response, cancellationToken).ConfigureAwait(false);
207+
}
208+
192209
public async Task<HttpResponseMessage> SendAsync(
193210
HttpRequestMessage request,
194211
CancellationToken cancellationToken = default)

0 commit comments

Comments
 (0)