dynamic_modules: make recreate_stream unsafe + add safe deferred wrapper

[bonnetn]

Commit Message

dynamic_modules: make recreate_stream unsafe + add safe deferred wrapper

EnvoyHttpFilter::recreate_stream() is a safe method today, but a successful call
synchronously destroys the current filter chain — freeing the Box<dyn HttpFilter>
that backs the running hook's self — before the FFI call returns. Because the
deallocation happens across the FFI boundary in Envoy's C++, the Rust borrow
checker cannot see it, so any use of self after the call is a use-after-free
reachable from ordinary safe Rust.

This change makes recreate_stream an unsafe fn with a # Safety contract
(matching the SDK's other caller-invariant FFI methods such as reset_http_stream,
send_http_stream_data, and set_filter_state_object), so safe Rust can no longer
reach the free. It also adds a safe request_stream_recreation(): the filter
records the intent, and the SDK performs the actual (freeing) recreation from the
event trampoline after the hook returns, when no reference to the filter remains.

Additional Description

The problem

Any access to self after the recreate_stream call reads/writes freed
memory. The borrow checker permits it because &mut self and &mut EnvoyHttpFilter
are disjoint borrows and the free happens in opaque C++.

Assume a Rust HTTP filter (dynamic module) that calls recreate_stream() upon
receiving request headers. The synchronous teardown runs entirely inside the call:

envoy_dynamic_module_on_http_filter_request_headers(envoy_ptr, filter_ptr, end_of_stream)  — http.rs:4451
│   let filter = filter_ptr as *mut *mut dyn HttpFilter<…>;  — http.rs:4457
│   let filter = &mut **filter;  // `self`: &mut dyn HttpFilter into the inner filter object — http.rs:4458
└─► HttpFilter::on_request_headers(&mut self, envoy, end_of_stream)   http.rs:4459
    ├─► EnvoyHttpFilter::recreate_stream(&mut self, None)  — http.rs:3974
    │   └─► envoy_dynamic_module_callback_http_filter_recreate_stream(raw_ptr, …)  — http.rs:3987
    │       └─► envoy_dynamic_module_callback_http_filter_recreate_stream(filter_envoy_ptr, …)  — abi_impl.cc:2766
    │           └─► StreamDecoderFilterCallbacks::recreateStream(response_headers)  — abi_impl.cc:2788
    │               └─► ActiveStreamDecoderFilter::recreateStream(const ResponseHeaderMap* headers)  — filter_manager.cc:1890
    │                   └─► FilterManagerCallbacks::recreateStream(streamInfo().filterState())  — filter_manager.cc:1917
    │                       └─► ConnectionManagerImpl::ActiveStream::recreateStream(FilterStateSharedPtr)  — conn_manager_impl.cc:2525
    │                           │   // ⚠️ "This functionally deletes the stream ... do not reference anything beyond this point." ⚠️
    │                           └─► ConnectionManagerImpl::doEndStream(*this, /*check_for_deferred_close=*/false)  — conn_manager_impl.cc:2547
    │                               └─► ConnectionManagerImpl::doEndStream(stream, check_for_deferred_close)  — conn_manager_impl.cc:269
    │                                   └─► ConnectionManagerImpl::doDeferredStreamDestroy(stream)  — call conn_manager_impl.cc:319, def conn_manager_impl.cc:331
    │                                       └─► FilterManager::destroyFilters()  — call conn_manager_impl.cc:387, def filter_manager.h:754
    │                                           └─► DynamicModuleHttpFilter::onDestroy()  — filter.cc:34
    │                                               └─► DynamicModuleHttpFilter::destroy()  — call filter.cc:44, def filter.cc:47
    │                                                   └─► config_->on_http_filter_destroy_(in_module_filter_)  — filter.cc:52
    │                                                       └─► envoy_dynamic_module_on_http_filter_destroy(filter_ptr)  — http.rs:4416
    │                                                           └─► Box::from_raw(*config) → drop(Box<dyn HttpFilter>)  — lib.rs:605
    │                                                               🚨🚨🚨 THE FREE: the inner filter object that `self` points at is deallocated here 🚨🚨🚨
    │
    │   ⬆ recreate_stream() returns all the way back up into the still-running on_request_headers hook,
    │     whose `&mut self` now dangles (the Box it points at was freed by the subtree above)
    │
    └─  self.marker = self.marker + 1;   🚨🚨🚨 USE-AFTER-FREE: read+write through the freed `self` — pure safe Rust 🚨🚨🚨

Minimal repro:

fn on_request_headers(&mut self, envoy: &mut EHF, _eos: bool) -> Status {
    envoy.recreate_stream(None);        // frees `self` (see callstack above)
    self.marker = self.marker + 1;      // safe Rust, yet `&self` points to freed memory
    Status::StopIteration
}

The fix

1. recreate_stream becomes unsafe fn with a # Safety contract documenting
   the synchronous cross-FFI teardown ("return StopIteration, never touch self
   again"). A compile_fail,E0133 doctest locks in that safe Rust cannot call it.
2. Safe request_stream_recreation() records the intent; the SDK performs the
   freeing recreate from the request-path trampoline after the hook returns, when
   the &mut self borrow is already gone.

Risk Level

Low. Rust dynamic-module SDK only; no data-plane or ABI change. The single change
in behavior for module authors is that recreate_stream now requires unsafe.

Testing

• //test/extensions/dynamic_modules:rust_sdk_doc_test — a compile_fail,E0133
  doctest on recreate_stream proves safe Rust cannot call it (fails to compile on
  the missing-unsafe-block error, on both the trait and the generated
  MockEnvoyHttpFilter).
• //test/extensions/dynamic_modules:rust_sdk_test — new
  test_request_stream_recreation_deferred_to_trampoline asserts the recreate FFI
  fires exactly once, only after the hook returns, and that the trampoline
  overrides the hook's Continue with StopIteration.
• //test/extensions/dynamic_modules:rust_sdk_clippy — clean.
• //test/extensions/dynamic_modules/http:integration_test
  (RecreateStreamNoUseAfterFree, --config=asan) — drives the safe deferred path
  through a real Envoy compiled with AddressSanitizer; passes for both rust and
  rust_static with no heap-use-after-free.

I mainly relied on AI to identify where these tests should go, so I would
appreciate guidance on whether they are written and located appropriately.

Docs Changes

N/A. The SDK is documented via rustdoc; the # Safety contract and the
request_stream_recreation doc comment are updated inline.

Release Notes

Added changelogs/current/behavior_changes/dynamic_modules__recreate-stream-unsafe.rst
noting the backward-incompatible SDK interface change (recreate_stream is now
unsafe) and the new safe request_stream_recreation.

Platform Specific Features

N/A.

Generative AI disclosure

Generative AI was used to help author this change and its description; the author
fully understands the code.

[repokitteh-read-only]

Hi @bonnetn, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

🐱

Caused by: #46367 was opened by bonnetn.

see: more, trace.

[bonnetn]

Note

Note to reviewers: I am following

envoy/CONTRIBUTING.md

Lines 154 to 157 in ca1d685

	* Any PR that changes user-facing behavior **must** have associated documentation in [docs](docs) as
	well as a release note fragment in [changelogs/current](changelogs/current). Add the fragment to
	the most appropriate section directory, using the canonical sections and areas listed in
	[changelogs/changelogs.yaml](changelogs/changelogs.yaml). Fragment filenames should use the form

This is not truly behavioral change, but this is backward incompatible interface change (adding unsafe).

[bonnetn]

Note

Note to the reviewers: I added recreate_requested field which is private, so we cant use EnvoyHttpFilterImpl { syntax anymore. I added a thin new constructor and updated these tests.

[bonnetn]

For visibility: this is something we discussed with @agrawroh / @basundhara-c

[basundhara-c]

Thanks for making this change! LGTM after resolving these two!

[basundhara-c]

@wbpcode could you help review this? Thanks in advance!
I think a deployment approval is also needed to trigger CI