feat: add skew protection to Frameworks API #6601
Conversation
|
This pull request adds or modifies JavaScript ( |
| @@ -8,6 +8,9 @@ export const FRAMEWORKS_API_CONFIG_ENDPOINT = `${FRAMEWORKS_API_ENDPOINT}/config | |||
| export const FRAMEWORKS_API_EDGE_FUNCTIONS_ENDPOINT = `${FRAMEWORKS_API_ENDPOINT}/edge-functions` | |||
| export const FRAMEWORKS_API_EDGE_FUNCTIONS_IMPORT_MAP = 'import_map.json' | |||
| export const FRAMEWORKS_API_FUNCTIONS_ENDPOINT = `${FRAMEWORKS_API_ENDPOINT}/functions` | |||
| export const FRAMEWORKS_API_SKEW_PROTECTION_ENDPOINT = `${FRAMEWORKS_API_ENDPOINT}/skew-protection.json` | |||
There was a problem hiding this comment.
I guess this is existing naming, but I find endpoint to be a misleading term here (especially since many of these do have very similar URL endpoints). how about "path"?
| try { | ||
| const data = await fs.readFile(configPath, 'utf8') | ||
| const config = validateSkewProtectionConfig(JSON.parse(data)) | ||
|
|
||
| return config | ||
| } catch (err) { | ||
| // If the file doesn't exist, this is a non-error. | ||
| if ((err as NodeJS.ErrnoException).code !== 'ENOENT') { | ||
| throw err | ||
| } |
There was a problem hiding this comment.
There are four cases here:
- exists + is json + valid
- exists + is json + invalid
- exists + is not json
- does not exist
We're testing 1, 2, and 4. Could we add a test for 3?
| if (error instanceof z.ZodError) { | ||
| throw new Error(`Invalid skew protection configuration: ${error.message}`) |
There was a problem hiding this comment.
Can we extend the assertion on the test for this to ensure the message contains actionable information? I think we actually need to read .errors for that: https://zod.dev/basics?id=handling-errors — otherwise we just get a generic validation error(?). Either way, test coverage would be valuable here.
|
|
||
| const validateSkewProtectionConfig = (data: unknown): SkewProtectionConfig => { | ||
| try { | ||
| return skewProtectionConfigSchema.parse(data) |
There was a problem hiding this comment.
nit: we could avoid the try-catch and conditional by using safeParse instead: https://zod.dev/basics?id=handling-errors
| @@ -8,6 +8,9 @@ export const FRAMEWORKS_API_CONFIG_ENDPOINT = `${FRAMEWORKS_API_ENDPOINT}/config | |||
| export const FRAMEWORKS_API_EDGE_FUNCTIONS_ENDPOINT = `${FRAMEWORKS_API_ENDPOINT}/edge-functions` | |||
| export const FRAMEWORKS_API_EDGE_FUNCTIONS_IMPORT_MAP = 'import_map.json' | |||
| export const FRAMEWORKS_API_FUNCTIONS_ENDPOINT = `${FRAMEWORKS_API_ENDPOINT}/functions` | |||
| export const FRAMEWORKS_API_SKEW_PROTECTION_ENDPOINT = `${FRAMEWORKS_API_ENDPOINT}/skew-protection.json` | |||
|
|
|||
| export const EDGE_REDIRECTS_DIST_PATH = '.netlify/deploy-config/edge-redirects.json' | |||
There was a problem hiding this comment.
I haven't found any trace of this in Linear, Notion, or Slack, but it seems to have been around for years in nfserver? Is this dead code we're reviving from the partially implemented and partially reverted edge redirects project from 2023...?
In any case, I'm not quite following what Skew Protection has to do with edge redirects. Would you mind explaining?
There was a problem hiding this comment.
Updated in cadd32d.
eduardoboucas
deleted the
eduardo/run-1845-add-frameworks-api-entry-point
branch
Summary
Closes https://linear.app/netlify/issue/RUN-1845/add-frameworks-api-entry-point.