Fix/endpoints issue #448
Fix/endpoints issue #448
Conversation
|
Coverage report for commit: ab52f72 Summary - Lines: 82.57% | Methods: 95.88% | Branches: 68.60%
🤖 comment via lucassabreu/comment-coverage-clover |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
test/sanity-check/api/oauth-test.js
Outdated
| developerHubHost = process.env.HOST.replace(/^dev\d*-api\./, 'dev-developerhub-api.') | ||
| } else if (process.env.HOST.startsWith('stag')) { | ||
| developerHubHost = process.env.HOST.replace(/^stag\d*-api\./, 'stag-developerhub-api.') | ||
| } else if (process.env.HOST.includes('contentstack.com') || process.env.HOST.includes('contentstack.io')) { |
Check failure
Code scanning / CodeQL
Incomplete URL substring sanitization
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
To fix this issue, we should avoid using a substring match on the host value. Instead, we should parse the host value and check that it matches an explicit whitelist of allowed hosts, or at least, that the host domain is exactly one of the intended domains (e.g., contentstack.com, contentstack.io), including allowed subdomains if needed.
We can use Node's native url module (URL class in modern JS) to safely parse the hostname of process.env.HOST and then check if it matches the allowed domains, either by exact match or by using a helper function that matches allowed subdomains.
Change only the code in file test/sanity-check/api/oauth-test.js at or around line 33 where the current substring check exists. Add import of Node's URL class if not available, and replace the .includes check with a check that gets the hostname and matches allowed values.
| @@ -30,9 +30,24 @@ | ||
| developerHubHost = process.env.HOST.replace(/^dev\d*-api\./, 'dev-developerhub-api.') | ||
| } else if (process.env.HOST.startsWith('stag')) { | ||
| developerHubHost = process.env.HOST.replace(/^stag\d*-api\./, 'stag-developerhub-api.') | ||
| } else if (process.env.HOST.includes('contentstack.com') || process.env.HOST.includes('contentstack.io')) { | ||
| // For standard Contentstack region hosts (e.g., au-api.contentstack.com -> au-developerhub-api.contentstack.com) | ||
| developerHubHost = process.env.HOST.replace(/-api\./, '-developerhub-api.') | ||
| } else { | ||
| // Safer domain check: for standard Contentstack region hosts (e.g., au-api.contentstack.com -> au-developerhub-api.contentstack.com) | ||
| try { | ||
| // Ensure HOST is a valid URL with protocol; if not, default to https for parsing | ||
| let hostToParse = process.env.HOST; | ||
| if (!/^https?:\/\//.test(hostToParse)) { | ||
| hostToParse = `https://${hostToParse}`; | ||
| } | ||
| const urlObj = new URL(hostToParse); | ||
| const hostname = urlObj.hostname; | ||
| // Allow only contentstack.com or contentstack.io or their subdomains | ||
| const allowedDomains = ["contentstack.com", "contentstack.io"]; | ||
| if (allowedDomains.some(d => hostname === d || hostname.endsWith('.' + d))) { | ||
| developerHubHost = process.env.HOST.replace(/-api\./, '-developerhub-api.'); | ||
| } | ||
| } catch (err) { | ||
| // Invalid HOST format; handle error or skip | ||
| } | ||
| } | ||
|
|
||
| if (developerHubHost) { |
test/sanity-check/api/oauth-test.js
Outdated
| developerHubHost = process.env.HOST.replace(/^dev\d*-api\./, 'dev-developerhub-api.') | ||
| } else if (process.env.HOST.startsWith('stag')) { | ||
| developerHubHost = process.env.HOST.replace(/^stag\d*-api\./, 'stag-developerhub-api.') | ||
| } else if (process.env.HOST.includes('contentstack.com') || process.env.HOST.includes('contentstack.io')) { |
Check failure
Code scanning / CodeQL
Incomplete URL substring sanitization
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
The fix is to avoid substring checks for hostnames and instead use proper host parsing and exact or suffix matching. On line 33, instead of checking if process.env.HOST.includes('contentstack.com') || process.env.HOST.includes('contentstack.io'), parse process.env.HOST as a hostname, then check if the hostname is exactly an allowed value or is a subdomain (with suffix matching starting on a dot). For Node.js, this can be achieved using the URL constructor for URLs, or with a regular expression if the value is always a bare hostname.
Since the code is working with just the hostname (not a full URL), we should check if the hostname ends with, for example, .contentstack.com or .contentstack.io or is exactly those domains.
Change:
process.env.HOST.includes('contentstack.com') || process.env.HOST.includes('contentstack.io')to a suffix check:
isAllowedContentstackHost(process.env.HOST)and define the helper:
function isAllowedContentstackHost(host) {
return (
host === 'contentstack.com' ||
host.endsWith('.contentstack.com') ||
host === 'contentstack.io' ||
host.endsWith('.contentstack.io')
);
}Ensure this helper is included near the top of the file.
| @@ -5,6 +5,17 @@ | ||
| import dotenv from 'dotenv' | ||
|
|
||
| dotenv.config() | ||
|
|
||
| // Safely check for allowed Contentstack hosts (not substring) | ||
| function isAllowedContentstackHost(host) { | ||
| return ( | ||
| host === 'contentstack.com' || | ||
| host.endsWith('.contentstack.com') || | ||
| host === 'contentstack.io' || | ||
| host.endsWith('.contentstack.io') | ||
| ); | ||
| } | ||
|
|
||
| let accessToken = '' | ||
| let loggedinUserID = '' | ||
| let authUrl = '' | ||
| @@ -30,7 +41,7 @@ | ||
| developerHubHost = process.env.HOST.replace(/^dev\d*-api\./, 'dev-developerhub-api.') | ||
| } else if (process.env.HOST.startsWith('stag')) { | ||
| developerHubHost = process.env.HOST.replace(/^stag\d*-api\./, 'stag-developerhub-api.') | ||
| } else if (process.env.HOST.includes('contentstack.com') || process.env.HOST.includes('contentstack.io')) { | ||
| } else if (isAllowedContentstackHost(process.env.HOST)) { | ||
| // For standard Contentstack region hosts (e.g., au-api.contentstack.com -> au-developerhub-api.contentstack.com) | ||
| developerHubHost = process.env.HOST.replace(/-api\./, '-developerhub-api.') | ||
| } |
harshithad0703
force-pushed
the
fix/endpoints-issue
branch
from
ab52f72 to
66c860b
Compare
No description provided.