Skip to content

Commit 6b5fc6f

Browse files
authored
feat: introduce HTTP policies for sandboxed HTTP Client (#968)
* feat: introduce HTTP policies for connection validation and enhance security testing * refactor(http): convert Policies fields to private to enforce encapsulation * refactor(http): remove package qualifier for `Policies` usage in tests to simplify code * refactor(http): simplify tests by removing package qualifier for client usage * refactor(http): consolidate Policies into Policy, add method and header validation, normalize API * refactor(http): replace fmt.Errorf with typed errors and add tests * refactor(http): update test context initialization and enhance error handling * refactor(http): introduce `MultiPolicyConfigurationError` for aggregated errors, enhance validation logic, and update tests * refactor(tests): enhance loopback HTTP policy tests with improved structure, explicit error checks, and added localhost opt-in validation * refactor(http): unify request handling by consolidating policy evaluation and preparation logic, improve validation and error handling, and update tests * refactor(http): ensure context is always initialized in `toStdRequest` to prevent `nil` context usage * refactor(http): remove redundant context initialization in `toStdRequest` * refactor(http): add nil check for `CloseIdleConnections` to improve safety * refactor(net): move `CloseIdleNetworkConnections` to `helpers.go` for better package organization and add nil check for improved safety
1 parent c6ed511 commit 6b5fc6f

63 files changed

Lines changed: 6560 additions & 403 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

engine.go

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"github.com/MontFerret/ferret/v2/pkg/bytecode"
99
"github.com/MontFerret/ferret/v2/pkg/bytecode/artifact"
1010
"github.com/MontFerret/ferret/v2/pkg/compiler"
11+
ferretnet "github.com/MontFerret/ferret/v2/pkg/net"
1112
"github.com/MontFerret/ferret/v2/pkg/source"
1213
"github.com/MontFerret/ferret/v2/pkg/vm"
1314
)
@@ -22,6 +23,7 @@ type Engine struct {
2223
limiter *sessionLimiter
2324
idleCap int
2425
totalCap int
26+
ownsNetwork bool
2527
}
2628

2729
// New constructs an Engine from the provided options, registers all modules,
@@ -33,14 +35,22 @@ func New(setters ...Option) (*Engine, error) {
3335
return nil, err
3436
}
3537

38+
ownsNetwork := opts.network == nil
39+
3640
boot, err := newBootstrap(opts)
3741
if err != nil {
3842
return nil, fmt.Errorf("bootstrap: %w", err)
3943
}
4044

4145
for _, m := range opts.modules {
4246
if err := m.Register(boot); err != nil {
43-
if closeErr := boot.hooks.engine.runCloseHooks(); closeErr != nil {
47+
closeErr := boot.hooks.engine.runCloseHooks()
48+
49+
if ownsNetwork {
50+
ferretnet.CloseIdleNetworkConnections(boot.host.Network())
51+
}
52+
53+
if closeErr != nil {
4454
return nil, errors.Join(err, fmt.Errorf("close hooks: %w", closeErr))
4555
}
4656

@@ -50,7 +60,13 @@ func New(setters ...Option) (*Engine, error) {
5060

5161
h, err := boot.host.Build()
5262
if err != nil {
53-
if closeErr := boot.hooks.engine.runCloseHooks(); closeErr != nil {
63+
closeErr := boot.hooks.engine.runCloseHooks()
64+
65+
if ownsNetwork {
66+
ferretnet.CloseIdleNetworkConnections(boot.host.Network())
67+
}
68+
69+
if closeErr != nil {
5470
return nil, errors.Join(err, fmt.Errorf("close hooks: %w", closeErr))
5571
}
5672

@@ -62,8 +78,13 @@ func New(setters ...Option) (*Engine, error) {
6278
// Run init hooks after bootstrap is finalized and before returning the engine.
6379
if err := hooks.engine.runInitHooks(); err != nil {
6480
initErr := fmt.Errorf("init hooks: %w", err)
81+
closeErr := hooks.engine.runCloseHooks()
6582

66-
if closeErr := hooks.engine.runCloseHooks(); closeErr != nil {
83+
if ownsNetwork {
84+
ferretnet.CloseIdleNetworkConnections(h.network)
85+
}
86+
87+
if closeErr != nil {
6788
return nil, errors.Join(initErr, fmt.Errorf("close hooks: %w", closeErr))
6889
}
6990

@@ -79,6 +100,7 @@ func New(setters ...Option) (*Engine, error) {
79100
limiter: newSessionLimiter(opts.maxActiveSessions),
80101
idleCap: opts.maxIdleVMsPerPlan,
81102
totalCap: opts.maxVMsPerPlan,
103+
ownsNetwork: ownsNetwork,
82104
}, nil
83105
}
84106

@@ -175,7 +197,13 @@ func (e *Engine) Run(ctx context.Context, src *source.Source, opts ...SessionOpt
175197

176198
// Close runs the engine close hooks and releases engine-scoped resources.
177199
func (e *Engine) Close() error {
178-
if err := e.hooks.engine.runCloseHooks(); err != nil {
200+
err := e.hooks.engine.runCloseHooks()
201+
202+
if e.ownsNetwork {
203+
ferretnet.CloseIdleNetworkConnections(e.host.network)
204+
}
205+
206+
if err != nil {
179207
return fmt.Errorf("close hooks: %w", err)
180208
}
181209

engine_lifecycle_test.go

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99

1010
"github.com/MontFerret/ferret/v2/pkg/logging"
1111
"github.com/MontFerret/ferret/v2/pkg/module"
12+
ferretnet "github.com/MontFerret/ferret/v2/pkg/net"
1213
"github.com/MontFerret/ferret/v2/pkg/runtime"
1314
"github.com/MontFerret/ferret/v2/pkg/source"
1415
"github.com/MontFerret/ferret/v2/pkg/vm"
@@ -50,10 +51,17 @@ func TestNewRunsCloseHooksWhenHostBuildFails(t *testing.T) {
5051
moduleRegistered bool
5152
closeHookCalled bool
5253
)
54+
client := &recordingHTTPClient{}
5355

5456
mod := testModule{
5557
registerFn: func(boot module.Bootstrap) error {
5658
moduleRegistered = true
59+
internal, ok := boot.(*bootstrap)
60+
if !ok {
61+
t.Fatalf("expected internal bootstrap, got %T", boot)
62+
}
63+
internal.host.network = mustNewTestNetwork(t, ferretnet.WithHTTPClient(client))
64+
5765
boot.Hooks().Engine().OnClose(func() error {
5866
closeHookCalled = true
5967
return nil
@@ -78,6 +86,10 @@ func TestNewRunsCloseHooksWhenHostBuildFails(t *testing.T) {
7886
if !closeHookCalled {
7987
t.Fatal("expected engine close hooks to run on host build failure")
8088
}
89+
90+
if got := client.idleCloseCount(); got != 1 {
91+
t.Fatalf("expected host-build-failure cleanup, got %d calls", got)
92+
}
8193
}
8294

8395
func TestNewReturnsJoinedErrorWhenInitAndCloseHooksFail(t *testing.T) {
@@ -115,6 +127,119 @@ func TestNewReturnsJoinedErrorWhenInitAndCloseHooksFail(t *testing.T) {
115127
}
116128
}
117129

130+
func TestEngineCloseClosesOwnedNetworkIdleConnections(t *testing.T) {
131+
t.Parallel()
132+
133+
client := &recordingHTTPClient{}
134+
eng := mustNewEngine(t)
135+
eng.host.network = mustNewTestNetwork(t, ferretnet.WithHTTPClient(client))
136+
137+
if err := eng.Close(); err != nil {
138+
t.Fatalf("close engine: %v", err)
139+
}
140+
141+
if got := client.idleCloseCount(); got != 1 {
142+
t.Fatalf("expected one idle-connection cleanup, got %d", got)
143+
}
144+
}
145+
146+
func TestEngineCloseCleansOwnedNetworkAfterHookFailure(t *testing.T) {
147+
t.Parallel()
148+
149+
hookErr := errors.New("close hook failed")
150+
client := &recordingHTTPClient{}
151+
eng := mustNewEngine(t, WithEngineCloseHook(func() error {
152+
return hookErr
153+
}))
154+
eng.host.network = mustNewTestNetwork(t, ferretnet.WithHTTPClient(client))
155+
156+
err := eng.Close()
157+
if !errors.Is(err, hookErr) {
158+
t.Fatalf("expected close hook error, got %v", err)
159+
}
160+
161+
if got := client.idleCloseCount(); got != 1 {
162+
t.Fatalf("expected cleanup after hook failure, got %d calls", got)
163+
}
164+
}
165+
166+
func TestEngineCloseDoesNotCleanInjectedNetwork(t *testing.T) {
167+
t.Parallel()
168+
169+
client := &recordingHTTPClient{}
170+
network := mustNewTestNetwork(t, ferretnet.WithHTTPClient(client))
171+
eng := mustNewEngine(t, WithNetwork(network))
172+
173+
if err := eng.Close(); err != nil {
174+
t.Fatalf("close engine: %v", err)
175+
}
176+
177+
if got := client.idleCloseCount(); got != 0 {
178+
t.Fatalf("expected injected network ownership to remain with caller, got %d cleanup calls", got)
179+
}
180+
}
181+
182+
func TestNewCleansOwnedNetworkOnRegistrationFailure(t *testing.T) {
183+
t.Parallel()
184+
185+
registerErr := errors.New("register failed")
186+
client := &recordingHTTPClient{}
187+
mod := testModule{
188+
registerFn: func(boot module.Bootstrap) error {
189+
internal, ok := boot.(*bootstrap)
190+
if !ok {
191+
t.Fatalf("expected internal bootstrap, got %T", boot)
192+
}
193+
194+
internal.host.network = mustNewTestNetwork(t, ferretnet.WithHTTPClient(client))
195+
196+
return registerErr
197+
},
198+
}
199+
200+
_, err := New(WithModules(mod))
201+
if !errors.Is(err, registerErr) {
202+
t.Fatalf("expected registration error, got %v", err)
203+
}
204+
205+
if got := client.idleCloseCount(); got != 1 {
206+
t.Fatalf("expected construction-failure cleanup, got %d calls", got)
207+
}
208+
}
209+
210+
func TestNewCleansOwnedNetworkOnInitFailure(t *testing.T) {
211+
t.Parallel()
212+
213+
initErr := errors.New("init failed")
214+
client := &recordingHTTPClient{}
215+
mod := testModule{
216+
registerFn: func(boot module.Bootstrap) error {
217+
internal, ok := boot.(*bootstrap)
218+
if !ok {
219+
t.Fatalf("expected internal bootstrap, got %T", boot)
220+
}
221+
222+
internal.host.network = mustNewTestNetwork(t, ferretnet.WithHTTPClient(client))
223+
224+
return nil
225+
},
226+
}
227+
228+
_, err := New(
229+
WithModules(mod),
230+
WithEngineInitHook(func() error {
231+
return initErr
232+
}),
233+
)
234+
if !errors.Is(err, initErr) {
235+
t.Fatalf("expected init error, got %v", err)
236+
}
237+
238+
if got := client.idleCloseCount(); got != 1 {
239+
t.Fatalf("expected init-failure cleanup, got %d calls", got)
240+
}
241+
}
242+
118243
func TestRunClosesPlanWhenSessionCreationFails(t *testing.T) {
119244
t.Parallel()
120245

host.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package ferret
22

33
import (
4+
"fmt"
5+
46
"github.com/MontFerret/ferret/v2/pkg/encoding"
57
"github.com/MontFerret/ferret/v2/pkg/fs"
68
"github.com/MontFerret/ferret/v2/pkg/logging"
@@ -37,7 +39,10 @@ func newHostContext(opts *options) (*hostContext, error) {
3739

3840
network := opts.network
3941
if network == nil {
40-
network = ferretnet.New()
42+
network, err = ferretnet.New()
43+
if err != nil {
44+
return nil, fmt.Errorf("network: %w", err)
45+
}
4146
}
4247

4348
return &hostContext{

network_context_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ func TestSessionRunInjectsConfiguredNetwork(t *testing.T) {
2626
t.Parallel()
2727

2828
client := &recordingHTTPClient{body: []byte("session-network")}
29-
engine := mustNewEngine(t, WithNetwork(ferretnet.New(ferretnet.WithHTTPClient(client))))
29+
network := mustNewTestNetwork(t, ferretnet.WithHTTPClient(client))
30+
engine := mustNewEngine(t, WithNetwork(network))
3031
defer func() { _ = engine.Close() }()
3132

3233
out, err := engine.Run(context.Background(), source.NewAnonymous(`
@@ -55,7 +56,8 @@ func TestDebugSessionRunInjectsConfiguredNetwork(t *testing.T) {
5556
t.Parallel()
5657

5758
client := &recordingHTTPClient{body: []byte("debug-network")}
58-
engine := mustNewEngine(t, WithNetwork(ferretnet.New(ferretnet.WithHTTPClient(client))))
59+
network := mustNewTestNetwork(t, ferretnet.WithHTTPClient(client))
60+
engine := mustNewEngine(t, WithNetwork(network))
5961
defer func() { _ = engine.Close() }()
6062

6163
plan, err := engine.CompileDebug(context.Background(), source.New("debug-network.fql", `

network_http_client_test.go

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,11 @@ import (
88
)
99

1010
type recordingHTTPClient struct {
11-
lastURL string
12-
body []byte
13-
calls int
14-
mu sync.Mutex
11+
lastURL string
12+
body []byte
13+
calls int
14+
idleCloses int
15+
mu sync.Mutex
1516
}
1617

1718
func (c *recordingHTTPClient) Do(_ context.Context, req *nethttp.Request) (*nethttp.Response, error) {
@@ -29,3 +30,17 @@ func (c *recordingHTTPClient) Do(_ context.Context, req *nethttp.Request) (*neth
2930
Body: append([]byte(nil), c.body...),
3031
}, nil
3132
}
33+
34+
func (c *recordingHTTPClient) CloseIdleConnections() {
35+
c.mu.Lock()
36+
defer c.mu.Unlock()
37+
38+
c.idleCloses++
39+
}
40+
41+
func (c *recordingHTTPClient) idleCloseCount() int {
42+
c.mu.Lock()
43+
defer c.mu.Unlock()
44+
45+
return c.idleCloses
46+
}

network_test_helpers_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package ferret
2+
3+
import (
4+
"testing"
5+
6+
ferretnet "github.com/MontFerret/ferret/v2/pkg/net"
7+
)
8+
9+
func mustNewTestNetwork(t testing.TB, setters ...ferretnet.Option) ferretnet.Network {
10+
t.Helper()
11+
12+
network, err := ferretnet.New(setters...)
13+
if err != nil {
14+
t.Fatalf("create test network: %v", err)
15+
}
16+
17+
return network
18+
}

pkg/net/context_test.go

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@ import (
77
)
88

99
func TestWithNetworkRoundTrip(t *testing.T) {
10-
network := New()
10+
network, err := New()
11+
if err != nil {
12+
t.Fatalf("create network: %v", err)
13+
}
1114
ctx := WithNetwork(context.Background(), network)
1215

1316
resolved, err := NetworkFrom(ctx)
@@ -22,7 +25,10 @@ func TestWithNetworkRoundTrip(t *testing.T) {
2225

2326
func TestHTTPClientFrom(t *testing.T) {
2427
client := stubHTTPClient{}
25-
network := New(WithHTTPClient(client))
28+
network, err := New(WithHTTPClient(client))
29+
if err != nil {
30+
t.Fatalf("create network: %v", err)
31+
}
2632
ctx := WithNetwork(context.Background(), network)
2733

2834
resolved, err := HTTPClientFrom(ctx)
@@ -48,3 +54,24 @@ func TestNetworkFromError(t *testing.T) {
4854
t.Fatalf("expected ErrNotFound for missing http client, got %v", err)
4955
}
5056
}
57+
58+
func TestDefaultNetworkForwardsIdleConnectionCleanup(t *testing.T) {
59+
t.Parallel()
60+
61+
client := &trackingHTTPClient{}
62+
network, err := New(WithHTTPClient(client))
63+
if err != nil {
64+
t.Fatalf("create network: %v", err)
65+
}
66+
closer, ok := network.(interface{ CloseIdleConnections() })
67+
if !ok {
68+
t.Fatalf("expected default network to expose idle-connection cleanup")
69+
}
70+
71+
closer.CloseIdleConnections()
72+
closer.CloseIdleConnections()
73+
74+
if got := client.idleCloseCount(); got != 2 {
75+
t.Fatalf("expected cleanup to be forwarded twice, got %d", got)
76+
}
77+
}

0 commit comments

Comments
 (0)