Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn on unconfigured cache #6545

Merged
merged 9 commits into from
Jun 8, 2022
Merged

Conversation

trevor-scheer
Copy link
Member

Issue a warning in production mode if neither the cache nor the APQ cache (persistedQueries.cache) are configured.

We've provided a simple path to using a bounded cache via:
#6536

The current default for AS3 is an unbounded in memory cache, which is susceptible to a DOS attack since APQs can fill up the server's memory with no limit. This warning provides an actionable recommendation to update their configuration in order to prevent this.

@trevor-scheer trevor-scheer requested a review from glasser June 8, 2022 18:06
@codesandbox-ci
Copy link

codesandbox-ci bot commented Jun 8, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 644d302:

Sandbox Source
Apollo Server Typescript Configuration
Apollo Server Configuration

@@ -261,6 +261,21 @@ export class ApolloServerBase<
: noIntro;
}

if (
this.config.nodeEnv === 'production' &&
Copy link
Member

Choose a reason for hiding this comment

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

!isDev

(requestOptions.persistedQueries === undefined ||
(requestOptions.persistedQueries &&
!requestOptions.persistedQueries.cache &&
!requestOptions.persistedQueries.ttl))
Copy link
Member

Choose a reason for hiding this comment

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

ttl not relevant

@@ -261,6 +261,21 @@ export class ApolloServerBase<
: noIntro;
}

if (
Copy link
Member

Choose a reason for hiding this comment

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

move to right before setting unboundedcache

Copy link
Member

Choose a reason for hiding this comment

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

not that it really matters but my idea was it could be nested inside the if (!requestOptions.cache) below instead of repeating that part of the condition

!requestOptions.persistedQueries.ttl))
) {
this.logger.warn(
'Apollo Server is running with an unbounded in-memory cache in production. ' +
Copy link
Member

Choose a reason for hiding this comment

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

Persisted queries are enabled and use an unbounded cache. Your server is vulnerable to denial of service attacks via memory exhaustion. Set cache: 'bounded' or persistedQueries: false in your ApolloServer constructor, or see DOCS for other alternatives.

Copy link
Member

@glasser glasser left a comment

Choose a reason for hiding this comment

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

oops, thought i approved before

@trevor-scheer trevor-scheer merged commit e08d765 into release-3.9.0 Jun 8, 2022
@trevor-scheer trevor-scheer deleted the trevor/cache-config-warning branch June 8, 2022 20:39
trevor-scheer added a commit that referenced this pull request Jun 15, 2022
Issue a warning in production mode if neither the cache nor the APQ cache
(persistedQueries.cache) are configured.

We've provided a simple path to using a bounded cache via: #6536

The current default for AS3 is an unbounded in memory cache, which is
susceptible to a DOS attack since APQs can fill up the server's memory with no
limit. This warning provides an actionable recommendation to update their
configuration in order to prevent this.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants