Skip to content

Added New Fields and Refactored Auto Generation#48

Merged
sachin-panayil merged 2 commits into
mainfrom
sachin/addignNewFields
Apr 30, 2025
Merged

Added New Fields and Refactored Auto Generation#48
sachin-panayil merged 2 commits into
mainfrom
sachin/addignNewFields

Conversation

@sachin-panayil

@sachin-panayil sachin-panayil commented Apr 24, 2025 •
edited by decause-gov
Loading

Copy link
Copy Markdown
Collaborator
  • Added upstream
  • Added forks within reuseFrequency
  • Refactored the auto-gen from a shallow merge of submissions and API data to individually changing fields
    • The shallow merge kept making random fields that were filled out blank and was not merging correctly so I changed it to populate each field individually
    • Was thinking of doing a deep merge but this felt complex asf and thought current method was more maintainable but I'm open to switching to deep merge

@sachin-panayil
added new fields and refactored auto gen
dea4170 
Signed-off-by: Sachin Panayil <sachinpanayil01@gmail.com>
@sachin-panayil sachin-panayil self-assigned this Apr 24, 2025
github-advanced-security[bot]
github-advanced-security AI found potential problems Apr 24, 2025
View reviewed changes
Comment thread js/autoGenerateFields.js Fixed
decause-gov
decause-gov approved these changes Apr 24, 2025
View reviewed changes

@decause-gov decause-gov left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Added my thoughts in various places. None of my comments are truly "blockers" but, we should perhaps file some issues for the things mentioned.

LGTM +1 🚢

Comment thread js/autoGenerateFields.js Outdated
Comment thread js/autoGenerateFields.js
Comment thread js/autoGenerateFields.js
This was referenced Apr 24, 2025
Open
Open
Closed
@sachin-panayil
added PR requested changes
9012cd6 
Signed-off-by: Sachin Panayil <sachinpanayil01@gmail.com>
github-advanced-security[bot]
github-advanced-security AI found potential problems Apr 30, 2025
View reviewed changes
Comment thread js/autoGenerateFields.js

// Update repositoryHost
if (repoData.html_url) {
if (repoData.html_url.includes('github.cms.gov')) {

Check failure

Code scanning / CodeQL

Incomplete URL substring sanitization

'[github.cms.gov](1)' can be anywhere in the URL, and arbitrary hosts may come before or after it.

Copilot Autofix

AI about 1 year ago

To fix the issue, the code should parse the URL using a reliable URL parser (e.g., the URL class in JavaScript) and validate the host property against a whitelist of allowed hosts. This ensures that the check is performed on the actual host component of the URL, rather than relying on substring matching, which is error-prone.

Specifically:

  1. Replace the includes checks with a parsed URL's host property.
  2. Use a whitelist of allowed hosts (e.g., ['github.cms.gov', 'github.com/CMSgov', 'github.com/CMS-Enterprise', 'github.com/DSACMS']) to validate the host.
  3. Update the logic to set the repositoryHost component only if the host matches one of the allowed values.
Suggested changeset 1
js/autoGenerateFields.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/js/autoGenerateFields.js b/js/autoGenerateFields.js
--- a/js/autoGenerateFields.js
+++ b/js/autoGenerateFields.js
@@ -267,10 +267,16 @@
         if (repoData.html_url) {
-            if (repoData.html_url.includes('github.cms.gov')) {
-                form.getComponent('repositoryHost').setValue('github.cms.gov')
-            } else if (repoData.html_url.includes('github.com/CMSgov')) {
-                form.getComponent('repositoryHost').setValue('github.com/CMSgov')
-            } else if (repoData.html_url.includes('github.com/CMS-Enterprise')) {
-                form.getComponent('repositoryHost').setValue('github.com/CMS-Enterprise')
-            } else if (repoData.html_url.includes('github.com/DSACMS')) {
-                form.getComponent('repositoryHost').setValue('github.com/DSACMS')
+            try {
+                const parsedUrl = new URL(repoData.html_url);
+                const allowedHosts = [
+                    'github.cms.gov',
+                    'github.com/CMSgov',
+                    'github.com/CMS-Enterprise',
+                    'github.com/DSACMS'
+                ];
+
+                if (allowedHosts.includes(parsedUrl.host)) {
+                    form.getComponent('repositoryHost').setValue(parsedUrl.host);
+                }
+            } catch (error) {
+                console.error("Invalid URL in repoData.html_url:", repoData.html_url, error);
             }
EOF
@@ -267,10 +267,16 @@
if (repoData.html_url) {
if (repoData.html_url.includes('github.cms.gov')) {
form.getComponent('repositoryHost').setValue('github.cms.gov')
} else if (repoData.html_url.includes('github.com/CMSgov')) {
form.getComponent('repositoryHost').setValue('github.com/CMSgov')
} else if (repoData.html_url.includes('github.com/CMS-Enterprise')) {
form.getComponent('repositoryHost').setValue('github.com/CMS-Enterprise')
} else if (repoData.html_url.includes('github.com/DSACMS')) {
form.getComponent('repositoryHost').setValue('github.com/DSACMS')
try {
const parsedUrl = new URL(repoData.html_url);
const allowedHosts = [
'github.cms.gov',
'github.com/CMSgov',
'github.com/CMS-Enterprise',
'github.com/DSACMS'
];

if (allowedHosts.includes(parsedUrl.host)) {
form.getComponent('repositoryHost').setValue(parsedUrl.host);
}
} catch (error) {
console.error("Invalid URL in repoData.html_url:", repoData.html_url, error);
}
Copilot is powered by AI and may make mistakes. Always verify output.
@sachin-panayil sachin-panayil merged commit a7d1a21 into main Apr 30, 2025
@sachin-panayil sachin-panayil deleted the sachin/addignNewFields branch April 30, 2025 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@decause-gov decause-gov decause-gov approved these changes

@IsaacMilarky IsaacMilarky Awaiting requested review from IsaacMilarky

@natalialuzuriaga natalialuzuriaga Awaiting requested review from natalialuzuriaga

@DinneK DinneK Awaiting requested review from DinneK

Assignees

@sachin-panayil sachin-panayil

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sachin-panayil @github-advanced-security @decause-gov