Skip to content

fix(apigatewayv1): enforce Smithy input constraints + improve round-trip probe#1410

Merged
vieiralucas merged 2 commits into
mainfrom
fix-apigwv1-100
May 16, 2026
Merged

fix(apigatewayv1): enforce Smithy input constraints + improve round-trip probe#1410
vieiralucas merged 2 commits into
mainfrom
fix-apigwv1-100

Conversation

@vieiralucas
Copy link
Copy Markdown
Member

@vieiralucas vieiralucas commented May 15, 2026

Summary

apigatewayv1 prevalidation: new crates/fakecloud-apigateway/src/validation.rs centralizes Smithy required-field, required-path-label, required-query, enum, and path-enum checks for all 36 control-plane operations the conformance probe negative-tests. Surfaces BadRequestException for missing/invalid inputs that handlers previously silently accepted.

Round-trip probe improvements (crates/fakecloud-conformance):

  • Forward every shared identifier into the follow-up Get/Describe so multi-segment REST URIs resolve.
  • Use the Create response to fill reader inputs whose names diverge from the writers (e.g. CreateModel.name -> GetModel.modelName, CreateAuthorizer response.id -> GetAuthorizer.authorizerId).
  • Isolate each round-trip variant under a unique resource identifier so concurrent variants do not clobber stored state.
  • Prefer the exact-suffix reader in find_round_trip_pairs (PutIntegrationResponse pairs with GetIntegrationResponse, not the shorter GetIntegration).

Conformance: apigatewayv1 3255/3375 (96.4%) -> 3376/3376 (100%).
Workspace baseline: 85115/86666 (98.2%) -> 85258/86679 (98.4%).

Test plan

  • cargo run -p fakecloud-conformance --release -- run --services apigatewayv1 reports 0 failures
  • cargo run -p fakecloud-conformance --release -- check --baseline conformance-baseline.json passes (no regressions across other services)
  • cargo clippy -p fakecloud-apigateway -p fakecloud-conformance --all-targets -- -D warnings
  • cargo test -p fakecloud-conformance --lib 83/83 pass
  • cargo fmt

Summary by cubic

Adds Smithy-aligned input validation to API Gateway v1 and strengthens the round-trip probe for multi-segment REST resources. apigatewayv1 now passes 100%, with a small workspace baseline bump.

  • New Features

    • crates/fakecloud-apigateway: Centralized prevalidation for 36 control-plane ops (required body/path/query, enum, path-enum, and required @httpPayload checks). Path labels are URL-decoded to catch {Name} placeholders. Emits BadRequestException before handlers run.
  • Bug Fixes

    • crates/fakecloud-conformance: Forward all shared identifiers to the follow-up Get; fill reader inputs from Create responses when names differ; isolate each round-trip with a unique string; prefer exact-suffix readers in find_round_trip_pairs.
    • crates/fakecloud-apigateway: Validation table fixes — RejectDomainNameAccessAssociation requires both domainNameAccessAssociationArn and domainNameArn query params; GetStages only requires restApiId (keep stageName for FlushStageCache/FlushStageAuthorizersCache).

Written for commit 3ae53c2. Summary will update on new commits. Review in cubic

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 8 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="crates/fakecloud-apigateway/src/validation.rs">

<violation number="1" location="crates/fakecloud-apigateway/src/validation.rs:326">
P1: `RejectDomainNameAccessAssociation` validates the wrong query parameter name (`domainNameArn`), so valid requests with `domainNameAccessAssociationArn` are rejected.</violation>

<violation number="2" location="crates/fakecloud-apigateway/src/validation.rs:358">
P1: `GetStages` is incorrectly validated as requiring `stageName`, causing valid `/restapis/{id}/stages` requests to return `BadRequestException`.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Re-trigger cubic

Comment thread crates/fakecloud-apigateway/src/validation.rs Outdated
Comment thread crates/fakecloud-apigateway/src/validation.rs Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented May 15, 2026

@vieiralucas
Copy link
Copy Markdown
Member Author

@cubic-dev-ai re-review

@cubic-dev-ai
Copy link
Copy Markdown

cubic-dev-ai Bot commented May 15, 2026

@cubic-dev-ai re-review

@vieiralucas I have started the AI code review. It will take a few minutes to complete.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 issues found across 8 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="crates/fakecloud-apigateway/src/validation.rs">

<violation number="1" location="crates/fakecloud-apigateway/src/validation.rs:262">
P2: `GetAuthorizer` is missing required `authorizerId` path validation because it shares constraints with `GetAuthorizers`.</violation>

<violation number="2" location="crates/fakecloud-apigateway/src/validation.rs:294">
P2: `DeleteDocumentationVersion` does not validate required `documentationVersion` path label.</violation>

<violation number="3" location="crates/fakecloud-apigateway/src/validation.rs:341">
P2: `DeleteModel` is missing required `modelName` path validation due shared constraints with `GetModels`.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Re-trigger cubic

Comment thread crates/fakecloud-apigateway/src/validation.rs Outdated
Comment thread crates/fakecloud-apigateway/src/validation.rs Outdated
Comment thread crates/fakecloud-apigateway/src/validation.rs Outdated
@vieiralucas
Copy link
Copy Markdown
Member Author

@cubic-dev-ai review

@cubic-dev-ai
Copy link
Copy Markdown

cubic-dev-ai Bot commented May 15, 2026

@cubic-dev-ai review

@vieiralucas I have started the AI code review. It will take a few minutes to complete.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 8 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="crates/fakecloud-apigateway/src/validation.rs">

<violation number="1" location="crates/fakecloud-apigateway/src/validation.rs:94">
P2: `@required` is enforced too strictly: empty string/list/map values are rejected for every required body field, which can incorrectly fail valid Smithy requests that only require presence/non-null.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Re-trigger cubic

Comment thread crates/fakecloud-apigateway/src/validation.rs
@vieiralucas vieiralucas force-pushed the fix-apigwv1-100 branch 2 times, most recently from d597f5c to 7d1b93c Compare May 15, 2026 23:38
@vieiralucas
Copy link
Copy Markdown
Member Author

@cubic-dev-ai review

@cubic-dev-ai
Copy link
Copy Markdown

cubic-dev-ai Bot commented May 16, 2026

@cubic-dev-ai review

@vieiralucas I have started the AI code review. It will take a few minutes to complete.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 8 files

Re-trigger cubic

@vieiralucas
Copy link
Copy Markdown
Member Author

@cubic-dev-ai review

@cubic-dev-ai
Copy link
Copy Markdown

cubic-dev-ai Bot commented May 16, 2026

@cubic-dev-ai review

@vieiralucas I have started the AI code review. It will take a few minutes to complete.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 8 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="crates/fakecloud-apigateway/src/service_rest_api_resources.rs">

<violation number="1" location="crates/fakecloud-apigateway/src/service_rest_api_resources.rs:23">
P1: The new global prevalidation call can incorrectly reject valid raw-payload import requests (ImportRestApi/ImportApiKeys) because validation expects a JSON `body` field instead of accepting HTTP payload bodies.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Re-trigger cubic

// body fields, missing/placeholder path labels, missing
// required query parameters, and invalid enum values before
// any per-handler logic runs.
crate::validation::prevalidate(action, req, &params)?;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: The new global prevalidation call can incorrectly reject valid raw-payload import requests (ImportRestApi/ImportApiKeys) because validation expects a JSON body field instead of accepting HTTP payload bodies.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/fakecloud-apigateway/src/service_rest_api_resources.rs, line 23:

<comment>The new global prevalidation call can incorrectly reject valid raw-payload import requests (ImportRestApi/ImportApiKeys) because validation expects a JSON `body` field instead of accepting HTTP payload bodies.</comment>

<file context>
@@ -15,6 +15,12 @@ impl ApiGatewayService {
+        // body fields, missing/placeholder path labels, missing
+        // required query parameters, and invalid enum values before
+        // any per-handler logic runs.
+        crate::validation::prevalidate(action, req, &params)?;
         match action {
             "GetAccount" => self.get_account(req),
</file context>

…rip probe

apigatewayv1 prevalidation
- New crates/fakecloud-apigateway/src/validation.rs centralizes Smithy
  required-field, required-path-label, required-query, enum, and
  path-enum checks for all 36 control-plane operations that the
  conformance probe negative-tests. Surfaces BadRequestException (the
  declared error shape) for missing required body/path/query fields
  and out-of-set enum values that handlers previously silently
  accepted.
- Path-label values are URL-decoded before the placeholder check so
  `{Name}` substrings sent literally by the probe (when it omits an
  @httpLabel member) are recognised as "missing".

Round-trip probe improvements (crates/fakecloud-conformance)
- Forward every shared identifier from the writer's input into the
  follow-up Get/Describe. Multi-segment REST resources (API Gateway's
  `/restapis/{r}/resources/{x}/methods/{m}`) need the whole composite
  key, not just `id_source_field`, otherwise the Get always 404s.
- Use the Create response to fill reader inputs whose names diverge
  from the writer's (e.g. CreateModel.name -> GetModel.modelName,
  CreateAuthorizer response.id -> GetAuthorizer.authorizerId). Match
  by exact field name first, then by stripped suffix heuristics
  (<Resource>Name -> response.name, <Resource>Id -> response.id).
- Isolate each round-trip variant under a unique resource identifier
  (`rt-<echo_field>`) so concurrent variants targeting the same writer
  can't clobber each other's stored state — prior behaviour produced
  spurious echo drops when a sibling variant overwrote the round-trip's
  PUT.
- Prefer the exact-suffix reader in `find_round_trip_pairs` so
  `PutIntegrationResponse` pairs with `GetIntegrationResponse` instead
  of the shorter-suffix `GetIntegration`.

Conformance: apigatewayv1 3255/3375 (96.4%) -> 3376/3376 (100%).
Workspace baseline: 85115/86666 (98.2%) -> 85258/86679 (98.4%).
- RejectDomainNameAccessAssociation now requires both
  domainNameAccessAssociationArn and domainNameArn as `@required`
  httpQuery members (Smithy declares both required).
- GetStages no longer claims `stageName` as a path label — the
  operation only takes `restApiId` from the path. Keep the requirement
  on FlushStageCache / FlushStageAuthorizersCache where stageName
  truly is part of the URI.

Conformance: still 3376/3376 (100%).
@vieiralucas
Copy link
Copy Markdown
Member Author

@cubic-dev-ai review

@cubic-dev-ai
Copy link
Copy Markdown

cubic-dev-ai Bot commented May 16, 2026

@cubic-dev-ai review

@vieiralucas I have started the AI code review. It will take a few minutes to complete.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 8 files

Re-trigger cubic

@vieiralucas vieiralucas merged commit 81f719a into main May 16, 2026
48 checks passed
@vieiralucas vieiralucas deleted the fix-apigwv1-100 branch May 16, 2026 02:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant