feat(runtime-interface-client): Add Lambda-Runtime-Invocation-Id support for cross-wiring protection#624
feat(runtime-interface-client): Add Lambda-Runtime-Invocation-Id support for cross-wiring protection#624vip-amzn wants to merge 1 commit into
Conversation
rudraroop
left a comment
There was a problem hiding this comment.
Looks good overall - my major point of feedback would be adding a test for the positive path where invocation IDs are non-null
Have added some casing/hardcoding related comments - whatever I could see
| headers.put(ERROR_TYPE_HEADER, error.errorType.getRapidError()); | ||
|
|
||
| if (invocationId != null) { | ||
| headers.put("Lambda-Runtime-Invocation-Id", invocationId); |
There was a problem hiding this comment.
the other headers in this file are declared as constants at the start of the file. I would suggest keeping that uniform and making a constant above for this one as well
|
|
||
| LambdaError lambdaError = new LambdaError(errorRequest, rapidErrorType); | ||
| lambdaRuntimeApiClientImpl.reportInvocationError(requestId, lambdaError); | ||
| lambdaRuntimeApiClientImpl.reportInvocationError(requestId, lambdaError, null); |
There was a problem hiding this comment.
I think we should have one test for reportInvocationError which tests a positive path i.e. one where the Lambda-Runtime-Invocation-Id is actually getting passed instead of null
There was a problem hiding this comment.
added reportInvocationErrorWithInvocationIdTest in LambdaRuntimeApiClientImplTest.java.
There was a problem hiding this comment.
Same feedback for positive path test inclusion as in this file LambdaRuntimeApiClientImplTest.java
There was a problem hiding this comment.
Added testInvocationIdIsPassedToReportError and testInvocationIdIsPassedToReportSuccess tests.
| headers = curl_slist_append(headers, "transfer-encoding:"); | ||
| headers = curl_slist_append(headers, m_user_agent_header.c_str()); | ||
| if (!invocation_id.empty()) { | ||
| headers = curl_slist_append(headers, ("lambda-runtime-invocation-id: " + invocation_id).c_str()); |
There was a problem hiding this comment.
Should use the header constant here instead of hardcoding again?
| headers.put(ERROR_TYPE_HEADER, error.errorType.getRapidError()); | ||
|
|
||
| if (invocationId != null) { | ||
| headers.put("Lambda-Runtime-Invocation-Id", invocationId); |
| @@ -41,6 +41,7 @@ static constexpr auto COGNITO_IDENTITY_HEADER = "lambda-runtime-cognito-identity | |||
| static constexpr auto DEADLINE_MS_HEADER = "lambda-runtime-deadline-ms"; | |||
There was a problem hiding this comment.
general question we need to investigate. Why does the jni module source is in a version tagged folder (aws-lambda-cpp-0.2.7), do we expect this to change in the current release model? Is this intentional?
This can also be interesting for @fabisev
| headers = curl_slist_append(headers, "transfer-encoding:"); | ||
| headers = curl_slist_append(headers, m_user_agent_header.c_str()); | ||
| if (!invocation_id.empty()) { | ||
| headers = curl_slist_append(headers, ("lambda-runtime-invocation-id: " + invocation_id).c_str()); |
|
|
||
| LambdaError lambdaError = new LambdaError(errorRequest, rapidErrorType); | ||
| lambdaRuntimeApiClientImpl.reportInvocationError(requestId, lambdaError); | ||
| lambdaRuntimeApiClientImpl.reportInvocationError(requestId, lambdaError, null); |
e259754 to
f7ed392
Compare
…ort for cross-wiring protection Echo the invocation ID received from RAPID on /next back on /response and /error, enabling RAPID to detect and reject stale responses from timed-out invocations. Fully backward compatible.
f7ed392 to
abe563d
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #624 +/- ##
=========================================
Coverage 65.38% 65.38%
- Complexity 211 212 +1
=========================================
Files 34 34
Lines 988 991 +3
Branches 142 143 +1
=========================================
+ Hits 646 648 +2
Misses 290 290
- Partials 52 53 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Summary
Add
Lambda-Runtime-Invocation-Idheader support for cross-wiring protection.The RIC now echoes the invocation ID received from RAPID on
/nextback on/responseand/error, enabling RAPID to detect and reject stale responses from timed-out invocations.Problem
On Lambda Managed Instances (LMI) and On-Demand (OD), when an invoke times out, the runtime process continues running in the background. If a new invoke arrives with the same
requestId, RAPID accepts it. The still-running old invocation eventually posts its response, and RAPID matches it to the new invoke — delivering the wrong response (cross-wiring).Solution
RAPID sends a unique per-invoke identifier via
Lambda-Runtime-Invocation-Idheader on/next. The runtime echoes it back on/responseand/error. RAPID validates the match before accepting the response.Changes
aws-lambda-cpp-0.2.7/runtime.h: Addedinvocation_idfield toinvocation_request, added param topost_success/post_failure/do_postaws-lambda-cpp-0.2.7/runtime.cpp: Parse header inget_next(), add to curl request indo_post()when non-emptyNativeClient.cpp/.h: JNI readsinvocation_idfrom C++ response, passes to Java;postInvocationResponseaccepts nullable invocationIdInvocationRequest.java: NewinvocationIdfieldNativeClient.java: Updated native method signatureLambdaRuntimeApiClient.java/Impl:reportInvocationSuccess/reportInvocationErroraccept invocationIdAWSLambda.java: Thread invocationId through runtime loopBackward Compatibility
Fully backward compatible in both directions: