EventGate Lambda entrypoint OOP #108
Conversation
tmikula-dev
requested review from
lsulak,
oto-macenauer-absa and
petr-pokorny-absa
as code owners
tmikula-dev
added
enhancement
|
AquaSec has completed a full security repository scan ✅ You can find the analysis results for this PR branch on this overview.
|
WalkthroughRefactors the Lambda entry to a ROUTE_MAP-driven dispatcher, adds HandlerApi to serve api.yaml, updates HandlerTopic to load access config from S3 or local sources and to handle requests, and adjusts tests to cover the new handlers and configuration paths. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Lambda
participant ROUTE_MAP
participant Handler
participant ConfigS3
Client->>Lambda: invoke with event/path
Lambda->>ROUTE_MAP: lookup handler for path
alt found
ROUTE_MAP-->>Lambda: handler reference
Lambda->>Handler: call handler.handle_request(event)
Handler->>ConfigS3: load access_config / schemas (S3 or local)
ConfigS3-->>Handler: return config/data
Handler->>Handler: validate, extract token, process
Handler-->>Lambda: response dict (statusCode, headers, body)
else not found
ROUTE_MAP-->>Lambda: no match
Lambda-->>Client: 404 response
end
Lambda-->>Client: return HTTP response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/event_gate_lambda.py`:
- Around line 79-87: The /terminate route in ROUTE_MAP currently maps to an
unprotected lambda that calls sys.exit("TERMINATING"); remove or guard this
endpoint: either delete the "/terminate" entry from ROUTE_MAP, or replace it
with a dedicated terminate handler that first checks an environment flag (e.g.,
ENABLE_TERMINATE or only allow in non-production) and validates a
secret/authorization token from the request before calling sys.exit; ensure the
new handler is the symbol referenced in ROUTE_MAP instead of the direct lambda
so authorization and env checks run prior to invoking sys.exit("TERMINATING").
In `@src/handlers/handler_topic.py`:
- Around line 114-125: The handler reads event["pathParameters"]["topic_name"],
event["body"], and calls json.loads without validating, which raises
KeyError/TypeError/JSONDecodeError; update the function handling this request to
validate inputs: check event is a dict, ensure pathParameters exists and
contains a non-empty topic_name (normalize with .lower()), verify httpMethod is
present and supported, and for POST ensure event["body"] is not None and is
valid JSON before calling json.loads; on any client-side issue return
build_error_response(400, "request", "<clear message>") and for unsupported
methods return 404 as before; keep references to topic_name, _get_topic_schema,
_post_topic_message, handler_token.extract_token, and build_error_response when
implementing these checks.
In `@tests/handlers/test_handler_api.py`:
- Around line 1-5: Add the standard 2026 copyright header to the top of the new
test file tests/handlers/test_handler_api.py so it includes the required project
header for new Python files created in 2026; modify the top of the file (above
the imports) to insert the exact project copyright block used across the repo
(year 2026) to satisfy header checks.
🧹 Nitpick comments (2)
tests/test_conf_validation.py (1)
62-64: Assertion is ineffective with@pytest.mark.parametrize.When the glob returns an empty list,
pytest.mark.parametrizewill simply skip the test (0 test cases generated) rather than fail it. Theassert topic_fileon line 64 will never execute if there are no files, so it cannot catch missing schema files.Consider using a separate test or fixture to validate that schema files exist:
💡 Suggested approach
def test_topic_schemas_directory_has_files(): """Ensure at least one topic schema exists.""" files = glob(os.path.join(CONF_DIR, "topic_schemas", "*.json")) assert files, "No *.json files found in topic_schemas/" `@pytest.mark.parametrize`("topic_file", glob(os.path.join(CONF_DIR, "topic_schemas", "*.json"))) def test_topic_json_schemas_basic(topic_file): schema = load_json(topic_file) # ... rest of validationsrc/handlers/handler_api.py (1)
51-63: Consider handling uninitialized state.
get_api()returns an empty body ifload_api_definition()was never called. While the current initialization flow inevent_gate_lambda.pychains these calls correctly, a defensive check could improve robustness.💡 Optional defensive check
def get_api(self) -> Dict[str, Any]: """ Return the OpenAPI specification. Returns: Dict[str, Any]: API Gateway response with OpenAPI spec. """ logger.debug("Handling GET API") + if not self.api_spec: + logger.warning("API spec not loaded; returning empty response") return { "statusCode": 200, "headers": {"Content-Type": "application/yaml"}, "body": self.api_spec, }
| # Route to handler function mapping | ||
| ROUTE_MAP: Dict[str, Any] = { | ||
| "/api": lambda _: handler_api.get_api(), | ||
| "/token": lambda _: handler_token.get_token_provider_info(), | ||
| "/health": lambda _: handler_health.get_health(), | ||
| "/topics": lambda _: handler_topic.get_topics_list(), | ||
| "/topics/{topic_name}": handler_topic.handle_request, | ||
| "/terminate": lambda _: sys.exit("TERMINATING"), | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, get the full context of the event_gate_lambda.py file
wc -l src/event_gate_lambda.pyRepository: AbsaOSS/EventGate
Length of output: 89
🏁 Script executed:
# Read the entire file to understand context
cat -n src/event_gate_lambda.pyRepository: AbsaOSS/EventGate
Length of output: 5115
🏁 Script executed:
# Search for authentication/authorization patterns
rg -n "auth" -i src/Repository: AbsaOSS/EventGate
Length of output: 1070
🏁 Script executed:
# Search for API Gateway configuration
find . -name "*.yaml" -o -name "*.yml" -o -name "*.json" | head -20 | xargs grep -l "terminate\|authorize\|auth" 2>/dev/null || echo "No matches in config files"Repository: AbsaOSS/EventGate
Length of output: 108
🏁 Script executed:
# Look for environment variable checks related to terminate or dev/prod
rg -n "environ|getenv|TERMINATE|DEV|PROD" src/event_gate_lambda.pyRepository: AbsaOSS/EventGate
Length of output: 111
🏁 Script executed:
# Search for the handler functions to understand flow
rg -n "def get_api|def get_token|def get_health|def get_topics|def handle_request" src/Repository: AbsaOSS/EventGate
Length of output: 488
🏁 Script executed:
# Check if there's a decorator or middleware wrapping the routes
rg -n "@|middleware|decorator" src/event_gate_lambda.pyRepository: AbsaOSS/EventGate
Length of output: 43
🏁 Script executed:
# Check api.yaml for authorization and endpoint config
cat api.yaml | head -100Repository: AbsaOSS/EventGate
Length of output: 2873
🏁 Script executed:
# Search for SAM template or CloudFormation config
find . -name "*.yaml" -o -name "*.yml" -o -name "*.json" | grep -E "(sam|cloudformation|template|serverless)" | head -10Repository: AbsaOSS/EventGate
Length of output: 43
🏁 Script executed:
# Search for test files that might test the terminate endpoint
find . -name "*test*.py" -o -name "test_*.py" | xargs grep -l "terminate" 2>/dev/null || echo "No test files found with 'terminate'"Repository: AbsaOSS/EventGate
Length of output: 97
🏁 Script executed:
# Search for any documentation or README mentioning terminate
rg -n "terminate" --type md --type txt . 2>/dev/null || echo "No docs found"Repository: AbsaOSS/EventGate
Length of output: 299
🏁 Script executed:
# Look for deployment configuration in .github workflows
find .github -name "*.yaml" -o -name "*.yml" 2>/dev/null | xargs catRepository: AbsaOSS/EventGate
Length of output: 40342
🏁 Script executed:
# Search for any config files that might define environment or feature flags
find . -name "*.env*" -o -name "config*.json" -o -name "settings*.py" | head -10 | xargs cat 2>/dev/null || echo "No config files found"Repository: AbsaOSS/EventGate
Length of output: 359
Remove or guard the /terminate endpoint.
The endpoint lacks authentication protection at the code level and can be called without credentials. While it's documented as "(internal)" for triggering cold starts, this lacks technical enforcement. An attacker with network access to the Lambda could repeatedly terminate instances, causing service disruption and increased cold start costs.
Recommended fixes:
- Guard the endpoint behind an environment variable check (enable only in development)
- Validate a secret/authorization token before allowing termination
- Remove the endpoint entirely and use Lambda lifecycle hooks or external orchestration for config reloads
🤖 Prompt for AI Agents
In `@src/event_gate_lambda.py` around lines 79 - 87, The /terminate route in
ROUTE_MAP currently maps to an unprotected lambda that calls
sys.exit("TERMINATING"); remove or guard this endpoint: either delete the
"/terminate" entry from ROUTE_MAP, or replace it with a dedicated terminate
handler that first checks an environment flag (e.g., ENABLE_TERMINATE or only
allow in non-production) and validates a secret/authorization token from the
request before calling sys.exit; ensure the new handler is the symbol referenced
in ROUTE_MAP instead of the direct lambda so authorization and env checks run
prior to invoking sys.exit("TERMINATING").
There was a problem hiding this comment.
This issue is not part of this PR, the potential issue was not created inside this PR.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check git history for when the /terminate endpoint was introduced
echo "=== Searching for 'terminate' in git history for src/event_gate_lambda.py ==="
git log -p --all -S "terminate" -- src/event_gate_lambda.py | head -100
echo ""
echo "=== Checking the file before this PR (master branch) ==="
git show master:src/event_gate_lambda.py | grep -n "terminate" -i -C 3 || echo "No 'terminate' found in master branch version"
echo ""
echo "=== Checking all commits that modified this file ==="
git log --oneline --all -- src/event_gate_lambda.py | head -10
Length of output: 1171
@tmikula-dev You're absolutely right—I apologize for the confusion. The /terminate endpoint already existed in the master branch (line 130) before this PR. Your refactoring simply moved the existing route handling from if-statements to the new ROUTE_MAP structure. The security concern, while potentially valid, is a pre-existing issue and not something introduced by your changes.
Thank you for the clarification! 🐰
/> />
(• ་ •)
| つ oops!
◡◡◡
Overview
This pull request refactors the EventGate Lambda entrypoint and related handler classes to improve modularity, testability, and maintainability. The main changes include extracting the OpenAPI handler into its own class, refactoring the topic handler.
Release Notes
Related
Closes #105
Summary by CodeRabbit
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.