[Major] Add server-side input validation #182
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Your Render PR Server URL is https://flashcards-pr-182.onrender.com. Follow its progress at https://dashboard.render.com/web/srv-cpnhmqhu0jms73fr1j20. |
Looking for library-provided sanitization instead.
dchege711
force-pushed
the
user/dchege711/database/library-backed-no-sql-injection-protection
branch
from
2848ed9 to
3009e3c
Compare
The NoSQL injection test fails with a 5XX error.
Depending on how we implement NoSQL injection protection, a tRPC-based test might not catch the fix.
I can't prove that this works though.
…lable cards The tests are still failing.
No new packages added, just moved around. zod was already a grand-dep of the @web/test-runner dev dependency.
Upgraded the package.
* Fetch a public card. * Return null for a private card. * Reject an empty body. * Reject a body without required params. * Resilient against extra parameters. * Return null for non-existent cards.
Seems like chaiAsPromised ships as an ESM, which I'm unable to use on
the server-side.
```
TypeError: Unknown file extension ".ts" for /Users/dchege711/study_buddy/src/routes/InAppRouter.test.ts
at Object.getFileProtocolModuleFormat [as file:] (node:internal/modules/esm/get_format:160:9)
at defaultGetFormat (node:internal/modules/esm/get_format:203:36)
at defaultLoad (node:internal/modules/esm/load:143:22)
at ModuleLoader.load (node:internal/modules/esm/loader:396:7)
at ModuleLoader.moduleProvider (node:internal/modules/esm/loader:278:45)
at link (node:internal/modules/esm/module_job:78:21)
```
This reverts commit e7fc6b7.
This reverts commit 2801bbe.
This reverts commit 76b9d45.
…esence of a validator Not all endpoints are surfaces for NoSQL injection.
`req.cookies.session_token` is typed as `any` and that's why TS did not catch this. The tests caught it though.
The account page using the browser's form POST action to request updates. We don't use the tRPC endpoint.
…umber
Search results were previously not showing because of:
```log
Uncaught (in promise) TRPCClientError: [
{
"code": "invalid_type",
"expected": "number",
"received": "null",
"path": [
"limit"
],
"message": "Expected number, received null"
}
]
at TRPCClientError.from (TRPCClientError.mjs:26:1)
at createHTTPBatchLink.mjs:59:59
```
For local development, the app is served through HTTP and the cookie ends up being not set. This makes logins not persist.
I've already sent 3,049 emails this month, which surpasses the 1K free emails quota. I'll be charged at least $1.00 + $2.05. A lot of these emails came from auto-initializing an account on dev server launch, and initializing an account for each test runs. These rack up pretty fast.
These clutter up the test run logs making it harder to identify real issues.
I can't yet verify that it helps. FWIW, it doesn't catch injection attacks on tRPC endpoints. Will re-install after validating its effectiveness.
dchege711
changed the title
dchege711
deleted the
user/dchege711/database/library-backed-no-sql-injection-protection
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Install
zod1 for use in input parsing/validation. Bumpvalidator,which we use in conjunction with
zod. We remove the customsanitizeQueryfunction.Some functional fixes discovered while validating/parsing input:
requireLogInmiddleware handles the case wherereq.session.useris not set (due to express session requiringHTTPS). It falls back to re-logging in the user through the session
token.
requireLogInmiddleware now passesreqinstead ofreq.cookies.session_tokento thelogInBySessionToken. Previously,this would have been a runtime error.
express-sessionuse thesecureoption in production but notin dev. [ES] Run local/test app in HTTPS mode to mirror production #185 tracks being able to use HTTPS in dev and tests.
nodemailer'ssendMailfunction in tests and devenvironments to avoid incurring costs from our email provider,
Mailgun.
Infinity,which serializes to
undefinedover the wire. We now send50instead.
timing out.