Skip to content

fix: validate REST API path segments in sam local start-api - #9356

Open
moneytool wants to merge 2 commits into
aws:developfrom
moneytool:fix/start-api-validate-rest-paths
Open

moneytool wants to merge 2 commits into
aws:developfrom
moneytool:fix/start-api-validate-rest-paths

Conversation

@moneytool

Copy link
Copy Markdown

Which issue(s) does this change fix?

#7613

Why is this change necessary?

sam local start-api serves Api event paths that API Gateway rejects on deploy, e.g. /api/map/{z}/{x}/{y}.png. The template works locally and then fails in CloudFormation with "Resource's path part only allow a-zA-Z0-9._-: or a valid greedy path variable and curly braces at the beginning and the end."

How does it address the issue?

SamApiProvider._convert_event_route now validates the Path of REST (Api) events. Each path part must be either a literal of a-zA-Z0-9._,:- (the character set documented for REST API path segments in Amazon API Gateway important notes) or a whole-part path variable ({name} or {name+}), and a greedy variable must be the last part. Paths that are intrinsic functions are not validated. Invalid paths raise InvalidSamDocumentException, the error the provider already uses for invalid event properties, which start-api reports as a user error.

Out of scope: HttpApi events, OpenAPI DefinitionBody paths, and CloudFormation AWS::ApiGateway::Resource paths are not validated.

What side effects does this change have?

Templates with REST Api event paths that API Gateway would reject now fail at sam local start-api startup instead of being served locally. Valid paths are unaffected.

Mandatory Checklist

PRs will only be reviewed after checklist is complete

  • Review the generative AI contribution guidelines (an AI coding assistant was used; I reviewed and tested the change)
  • Add input/output type hints to new functions/methods
  • Write design document if needed (Do I need to write a design document?) (not needed: bug fix)
  • Write/update unit tests
  • Write/update integration tests
  • Write/update functional tests if needed
  • make pr passes
  • make update-reproducible-reqs if dependencies were changed (no dependency changes)
  • Write documentation

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@moneytool
moneytool requested a review from a team as a code owner October 21, 2026 04:02
@github-actions github-actions Bot added pr/external stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at. labels Oct 21, 2026
@aws-sam-tooling-bot

Copy link
Copy Markdown

🧪 Hands-on test plan (not run: hands-on runs are disabled)

Commit ba2ee96. This plan was not run, so nothing in it has been tested.

Summary: Run sam local start-api on small SAM templates. Invalid REST Api paths (the issue's example, a greedy variable that isn't last, unicode or space segments, a parameter-resolved path) should fail fast with a clear Error and exit code 1. Valid paths should still mount and invoke, and HttpApi paths should stay unvalidated. Compare with the merge base, where the invalid paths are served, and run the provider unit tests.

Why: The PR changes what users see from sam local start-api: REST Api event paths that API Gateway rejects now fail at startup with a user error, where before they were served. This is easy to show with small templates, and comparing against the merge base confirms the fix. The change is a pure regex check with no new dependencies, network calls, install hooks or CI/agent-file changes, so a run is safe. Docker is needed because start-api creates its container client before route extraction reaches the new check.

Needs Docker: yes

Needs cloud resources (would not be run):

  • Deploying templates with paths the new local check accepts (/a//b, /{a-b}, /items/a,b, /items/v1:batch-get) to confirm API Gateway also accepts them
  • Deploying /api/map/{z}/{x}/{y}.png and /{proxy+}/items to confirm CloudFormation rejects exactly the paths the new check rejects

Setup:

python -m venv .venv
. .venv/bin/activate && SAM_CLI_DEV=1 pip install -e '.[dev]'
mkdir -p /tmp/ho/app && printf '%s\n' 'def handler(event, context):' '    return {"statusCode": 200, "body": event["path"]}' > /tmp/ho/app/app.py
printf '%s\n' 'AWSTemplateFormatVersion: "2010-09-09"' 'Transform: AWS::Serverless-2016-10-31' 'Resources:' '  MapFunction:' '    Type: AWS::Serverless::Function' '    Properties:' '      CodeUri: /tmp/ho/app' '      Handler: app.handler' '      Runtime: python3.12' '      Events:' > /tmp/ho/head.yaml

Scenarios (8)

S1: Issue #‍7613 repro: an Api event path with a partial-segment variable ({y}.png) should now be rejected at start-api startup with a user error (compared with the base branch)

mkdir -p /tmp/ho/s1 && { cat /tmp/ho/head.yaml; printf '%s\n' '        E1:' '          Type: Api' '          Properties:' '            Path: "/api/map/{z}/{x}/{y}.png"' '            Method: get'; } > /tmp/ho/s1/template.yaml
. .venv/bin/activate && cd /tmp/ho/s1 && SAM_CLI_TELEMETRY=0 timeout 90 sam local start-api -t template.yaml --port 3011 > out.log 2>&1; echo "exit=$?"; cat out.log

Expected: PR: exit=1; out.log contains "Error: Invalid Path '/api/map/{z}/{x}/{y}.png' in Api event of function 'MapFunction': Resource's path part '{y}.png' only allows a-zA-Z0-9._,:-"; no 'Mounting' line and no Python traceback. Merge base: no error, a line 'Mounting MapFunction at hxxp://127.0.0.1:3011/ap…

S2: Valid REST paths still mount and invoke: path variables, a literal segment after a variable, a colon in a literal, and a trailing greedy variable

mkdir -p /tmp/ho/s2 && { cat /tmp/ho/head.yaml; printf '%s\n' '        E1:' '          Type: Api' '          Properties:' '            Path: "/users/{id}"' '            Method: get' '        E2:' '          Type: Api' '          Properties:' '            Path: "/files/{name}/data.json"' '            Method: get'; } > /tmp/ho/s2/template.yaml
printf '%s\n' '        E3:' '          Type: Api' '          Properties:' '            Path: "/items/v1:batch-get"' '            Method: get' '        E4:' '          Type: Api' '          Properties:' '            Path: "/api/{version}/{proxy+}"' '            Method: get' >> /tmp/ho/s2/template.yaml
. .venv/bin/activate && cd /tmp/ho/s2 && (SAM_CLI_TELEMETRY=0 sam local start-api -t template.yaml --port 3012 > out.log 2>&1 & echo $! > pid) && for i in $(seq 120); do grep -qE 'Running on|Error' out.log && break; sleep 1; done; cat out.log
for p in /users/42 /files/report/data.json /items/v1:batch-get /api/v2/a/b/c; do curl -s -m 240 -w " code=%{http_code}\n" "http://127.0.0.1:3012$p"; done
kill $(cat /tmp/ho/s2/pid); sleep 2; cat /tmp/ho/s2/out.log | tail -n 30

Expected: out.log shows four 'Mounting MapFunction at hxxp://127.0.0.1:3012/...' lines (users/{id}, files/{name}/data.json, items/v1:batch-get, api/{version}/{proxy+}) and no 'Error'. Each curl prints a body equal to the requested path followed by code=200. Behavior matches the merge base.

S3: New error path: a greedy variable that is not the last segment (compared with the base branch)

mkdir -p /tmp/ho/s3 && { cat /tmp/ho/head.yaml; printf '%s\n' '        E1:' '          Type: Api' '          Properties:' '            Path: "/{proxy+}/items"' '            Method: get'; } > /tmp/ho/s3/template.yaml
. .venv/bin/activate && cd /tmp/ho/s3 && SAM_CLI_TELEMETRY=0 timeout 90 sam local start-api -t template.yaml --port 3013 > out.log 2>&1; echo "exit=$?"; cat out.log

Expected: PR: exit=1; out.log contains "Error: Invalid Path '/{proxy+}/items' in Api event of function 'MapFunction': greedy path variable '{proxy+}' must be the last part of the path." and no traceback. Merge base: the route is mounted (or fails differently); record what the base does.

S4: Out-of-scope check: an HttpApi event with the same partial-segment variable is still not validated and starts as before

mkdir -p /tmp/ho/s4 && { cat /tmp/ho/head.yaml; printf '%s\n' '        E1:' '          Type: HttpApi' '          Properties:' '            Path: "/api/map/{y}.png"' '            Method: get'; } > /tmp/ho/s4/template.yaml
. .venv/bin/activate && cd /tmp/ho/s4 && SAM_CLI_TELEMETRY=0 timeout 60 sam local start-api -t template.yaml --port 3014 > out.log 2>&1; echo "exit=$?"; cat out.log

Expected: exit=124 (server kept running until timeout); out.log contains 'Mounting MapFunction at hxxp://127.0.0.1:3014/api/map/{y}.png [GET]' and no 'Invalid Path' error.

S5: Path set with !Ref to a parameter: check whether the resolved string is validated (the PR says intrinsic paths are skipped) and that a valid default still starts (compared with the base branch)

mkdir -p /tmp/ho/s5 && { cat /tmp/ho/head.yaml; printf '%s\n' '        E1:' '          Type: Api' '          Properties:' '            Path: !Ref ApiPath' '            Method: get' 'Parameters:' '  ApiPath:' '    Type: String' '    Default: /ok'; } > /tmp/ho/s5/template.yaml
. .venv/bin/activate && cd /tmp/ho/s5 && SAM_CLI_TELEMETRY=0 timeout 90 sam local start-api -t template.yaml --port 3015 --parameter-overrides 'ApiPath=/bad/{id}.png' > bad.log 2>&1; echo "exit=$?"; cat bad.log
. .venv/bin/activate && cd /tmp/ho/s5 && SAM_CLI_TELEMETRY=0 timeout 60 sam local start-api -t template.yaml --port 3015 > ok.log 2>&1; echo "exit=$?"; cat ok.log

Expected: First run (PR): exit=1 with "Invalid Path '/bad/{id}.png'" and path part '{id}.png', since the parameter is resolved before route extraction. If it instead mounts /bad/{id}.png, record that parameter-driven paths bypass validation. Second run: exit=124 and 'Mounting MapFunction at hxxp://127.0.0.1:…

S6: Unicode and space segments: rejected with a readable message, no encoding errors or tracebacks (compared with the base branch)

mkdir -p /tmp/ho/s6 && { cat /tmp/ho/head.yaml; printf '%s\n' '        E1:' '          Type: Api' '          Properties:' '            Path: "/café/hello world"' '            Method: get'; } > /tmp/ho/s6/template.yaml
. .venv/bin/activate && cd /tmp/ho/s6 && SAM_CLI_TELEMETRY=0 timeout 90 sam local start-api -t template.yaml --port 3016 > out.log 2>&1; echo "exit=$?"; cat out.log

Expected: PR: exit=1; out.log contains "Invalid Path '/café/hello world'" and "path part 'café'" with the é intact; no UnicodeEncodeError and no traceback. Merge base: the route is mounted (exit=124) or fails differently; record it.

S7: Lenient edge cases the regex accepts: an empty segment (/a//b), a dash in a variable name (/{a-b}), a comma literal (/items/a,b). Record the local behavior; whether API Gateway accepts them is in needs_cloud (compared with the base branch)

mkdir -p /tmp/ho/s7 && { cat /tmp/ho/head.yaml; printf '%s\n' '        E1:' '          Type: Api' '          Properties:' '            Path: "/a//b"' '            Method: get' '        E2:' '          Type: Api' '          Properties:' '            Path: "/{a-b}"' '            Method: get'; } > /tmp/ho/s7/template.yaml
printf '%s\n' '        E3:' '          Type: Api' '          Properties:' '            Path: "/items/a,b"' '            Method: get' >> /tmp/ho/s7/template.yaml
. .venv/bin/activate && cd /tmp/ho/s7 && SAM_CLI_TELEMETRY=0 timeout 60 sam local start-api -t template.yaml --port 3017 > out.log 2>&1; echo "exit=$?"; cat out.log

Expected: PR: no 'Invalid Path' error for any of the three paths; the result matches the merge base (expected exit=124 with three Mounting lines). Any traceback, such as from Flask rule conversion of {a-b}, should match the base; a difference from the base is a regression.

S8: Unit tests for the provider, the new validation tests and the start-api wiring pass with the PR

. .venv/bin/activate && python -m pytest tests/unit/commands/local/lib/test_sam_api_provider.py tests/unit/commands/local/lib/test_api_provider.py tests/unit/commands/local/lib/test_local_api_service.py tests/unit/commands/local/start_api -q -p no:cacheprovider

Expected: Exit code 0. All tests pass, including TestSamApiRestApiPathValidation (9 valid, 6 invalid, 1 HttpApi case). No existing test in test_sam_api_provider.py fails because its Api path is now rejected.

🤖 Automated hands-on test — not a review approval · model claude-opus-5.5 · commit ba2ee96

History (1)
Commit Result When (UTC) Build
ba2ee96 Plan only (not run) 2026-10-22 23:08

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr/external stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants