[Feature] New submissions section on VaultDetailsPage #551
Conversation
|
WalkthroughThis pull request introduces significant enhancements to the Changes
Uplevel your code reviews with CodeRabbit ProCodeRabbit ProIf you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, CodeRabbit Pro is free for open source projects. |
Deploying with
|
| Latest commit: |
0395c13
|
| Status: | ⚡️ Build in progress... |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Files selected (24)
- packages/web/src/pages/Honeypots/VaultDetailsPage/Sections/VaultScopeSection/InScopeSection/InScopeSection.tsx (3)
- packages/web/src/pages/Honeypots/VaultDetailsPage/Sections/VaultSubmissionsSection/PublicSubmissionCard/PublicSubmissionCard.tsx (1)
- packages/web/src/pages/Honeypots/VaultDetailsPage/Sections/VaultSubmissionsSection/PublicSubmissionCard/styles.ts (1)
- packages/web/src/pages/Honeypots/VaultDetailsPage/Sections/VaultSubmissionsSection/VaultSubmissionsSection.tsx (1)
- packages/web/src/pages/Honeypots/VaultDetailsPage/Sections/VaultSubmissionsSection/styles.ts (1)
- packages/web/src/pages/Honeypots/VaultDetailsPage/Sections/index.ts (1)
- packages/web/src/pages/Honeypots/VaultDetailsPage/VaultDetailsPage.tsx (5)
- packages/web/src/pages/Honeypots/VaultDetailsPage/hooks.ts (1)
- packages/web/src/pages/Honeypots/VaultDetailsPage/savedSubmissionsService.ts (1)
- packages/web/src/pages/Honeypots/VaultDetailsPage/styles.ts (2)
- packages/web/src/pages/Honeypots/VaultDetailsPage/types.ts (1)
- packages/web/src/pages/Submissions/SubmissionFormPage/FormSteps/SubmissionContactInfo/SubmissionContactInfo.tsx (4)
- packages/web/src/pages/Submissions/SubmissionFormPage/FormSteps/SubmissionDescriptions/SubmissionDescriptions.tsx (8)
- packages/web/src/pages/Submissions/SubmissionFormPage/FormSteps/SubmissionDescriptions/utils.ts (1)
- packages/web/src/pages/Submissions/SubmissionFormPage/FormSteps/SubmissionProject/SubmissionProject.tsx (2)
- packages/web/src/pages/Submissions/SubmissionFormPage/FormSteps/SubmissionSubmit/components/SubmissionProcessed/SubmissionProcessed.tsx (1)
- packages/web/src/pages/Submissions/SubmissionFormPage/FormSteps/SubmissionSubmit/components/SubmissionReview/SubmissionReview.tsx (2)
- packages/web/src/pages/Submissions/SubmissionFormPage/FormSteps/SubmissionSubmit/components/SubmissionReview/styles.ts (1)
- packages/web/src/pages/Submissions/SubmissionFormPage/SubmissionFormPage.tsx (5)
- packages/web/src/pages/Submissions/SubmissionFormPage/store.ts (1)
- packages/web/src/pages/Submissions/SubmissionFormPage/styles.ts (2)
- packages/web/src/pages/Submissions/SubmissionFormPage/submissionsService.api.ts (1)
- packages/web/src/pages/Submissions/SubmissionFormPage/types.ts (1)
- packages/web/src/utils/contractsCovered.utils.ts (1)
Files ignored due to filter (2)
- packages/web/src/assets/images/audit_wizard.png
- packages/web/src/languages/en.json
Files skipped from review due to trivial changes (13)
- packages/web/src/pages/Honeypots/VaultDetailsPage/Sections/VaultSubmissionsSection/PublicSubmissionCard/PublicSubmissionCard.tsx
- packages/web/src/pages/Honeypots/VaultDetailsPage/Sections/VaultSubmissionsSection/PublicSubmissionCard/styles.ts
- packages/web/src/pages/Honeypots/VaultDetailsPage/Sections/VaultSubmissionsSection/styles.ts
- packages/web/src/pages/Honeypots/VaultDetailsPage/hooks.ts
- packages/web/src/pages/Honeypots/VaultDetailsPage/styles.ts
- packages/web/src/pages/Honeypots/VaultDetailsPage/types.ts
- packages/web/src/pages/Submissions/SubmissionFormPage/FormSteps/SubmissionSubmit/components/SubmissionProcessed/SubmissionProcessed.tsx
- packages/web/src/pages/Submissions/SubmissionFormPage/FormSteps/SubmissionSubmit/components/SubmissionReview/styles.ts
- packages/web/src/pages/Submissions/SubmissionFormPage/store.ts
- packages/web/src/pages/Submissions/SubmissionFormPage/styles.ts
- packages/web/src/pages/Submissions/SubmissionFormPage/submissionsService.api.ts
- packages/web/src/pages/Submissions/SubmissionFormPage/types.ts
- packages/web/src/utils/contractsCovered.utils.ts
Review comments generated (35)
- Review: 25
- LGTM: 10
Tips
Chat with 
CodeRabbit Bot (@coderabbitai)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitaiin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
| return `${submissionData.ref === "audit-wizard" ? "***Submitted via auditwizard.io***\n" : ""} | ||
| **Github username:** ${submissionData.contact?.githubUsername ? `@${submissionData.contact?.githubUsername}` : "--"} | ||
| **Submission hash (on-chain):** ${submissionData.submissionResult?.transactionHash} | ||
| **Severity:** ${description.severity} |
There was a problem hiding this comment.
The changes made here are good for providing additional information about the source of the submission. However, it's important to handle potential null or undefined values to prevent possible runtime errors. For instance, description.severity could potentially be undefined. Consider adding a fallback value or a check before accessing this property.
- **Severity:** ${description.severity}
+ **Severity:** ${description?.severity || 'N/A'}This will ensure that if description or severity is not defined, 'N/A' will be displayed instead of causing an error.
| export async function getSavedSubmissions(vaultId: string | undefined): Promise<IGithubIssue[]> { | ||
| if (!vaultId) return []; | ||
|
|
||
| try { | ||
| const response = await axiosClient.get(`${BASE_SERVICE_URL}/github-repos/issues/${vaultId}`); | ||
| const githubIssues = response.data.githubIssues as IGithubIssue[]; | ||
| const githubIssuesWithSeverity = githubIssues.map((issue) => { | ||
| const severity = issue.issueData.issueDescription.match(/(\*\*Severity:\*\* (.*)\n)/)?.[2]; | ||
| return { | ||
| ...issue, | ||
| severity, | ||
| }; | ||
| }); | ||
|
|
||
| return githubIssuesWithSeverity; | ||
| } catch (error) { | ||
| return []; | ||
| } |
There was a problem hiding this comment.
The function getSavedSubmissions does not handle errors properly. It simply returns an empty array when an error occurs, which might be misleading for the caller of this function. Instead, it would be better to throw the error and let the caller decide how to handle it.
- } catch (error) {
- return [];
- }
+ } catch (error) {
+ throw error;
+ }| export async function getVaultRepoName(vaultId: string | undefined): Promise<string | undefined> { | ||
| if (!vaultId) return undefined; | ||
|
|
||
| try { | ||
| const response = await axiosClient.get(`${BASE_SERVICE_URL}/github-repos/repo/${vaultId}`); | ||
| const repoName = response.data.repoName as string | undefined; | ||
| console.log(response); | ||
|
|
||
| return repoName; | ||
| } catch (error) { | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
Similar to the previous comment, the function getVaultRepoName should throw the error instead of returning undefined. Also, there is a console.log statement on line 37 that should be removed as it could potentially leak sensitive information.
- } catch (error) {
- return undefined;
- }
+ } catch (error) {
+ throw error;
+ }
- console.log(response);| @@ -13,7 +13,7 @@ import TerminalIcon from "@mui/icons-material/Terminal"; | |||
| import ContractsIcon from "@mui/icons-material/ViewInAr"; | |||
| import ContractsTwoIcon from "@mui/icons-material/ViewWeekOutlined"; | |||
| import MDEditor from "@uiw/react-md-editor"; | |||
| import { Alert, Button, CopyToClipboard, Pill, WithTooltip } from "components"; | |||
| import { Alert, Button, CopyToClipboard, Loading, Pill, WithTooltip } from "components"; | |||
There was a problem hiding this comment.
The import statement seems to be unchanged except for the addition of line numbers. There's no issue with this line.
- import { Alert, Button, CopyToClipboard, Pill, WithTooltip } from "components";
+ import { Alert, Button, CopyToClipboard, Loading, Pill, WithTooltip } from "components";| @@ -27,7 +27,7 @@ import { getAuditSubmissionTexts, getBountySubmissionTexts } from "./utils"; | |||
| export function SubmissionDescriptions() { | |||
| const { t } = useTranslation(); | |||
|
|
|||
| const { submissionData, setSubmissionData, vault } = useContext(SubmissionFormContext); | |||
| const { submissionData, setSubmissionData, vault, allFormDisabled } = useContext(SubmissionFormContext); | |||
There was a problem hiding this comment.
The addition of allFormDisabled in the context is a good approach to control the state of the form. However, it's important to ensure that this variable is properly initialized and updated in the SubmissionFormContext to avoid any unexpected behavior.
- const { submissionData, setSubmissionData, vault, allFormDisabled } = useContext(SubmissionFormContext);
+ const { submissionData, setSubmissionData, vault, allFormDisabled = false } = useContext(SubmissionFormContext);| @@ -97,6 +100,7 @@ export function SubmissionContactInfo() { | |||
| <p className="mb-2">{t("Submissions.addGithubAccountConnectIssue")}</p> | |||
| <FormInput | |||
| {...register("githubUsername")} | |||
| disabled={allFormDisabled} | |||
There was a problem hiding this comment.
Disabling the GitHub username input when allFormDisabled is true is a good practice. It prevents users from changing their input during processing or loading states. Ensure that the allFormDisabled flag is being managed correctly in your state logic.
- disabled={allFormDisabled}
+ disabled={allFormDisabled || false}| <br /> | ||
|
|
||
| <MDEditor.Markdown | ||
| disallowedElements={disallowedElementsMarkdown} | ||
| source={submissionData?.submissionsDescriptions?.submissionMessage} | ||
| style={{ whiteSpace: "normal", fontSize: "var(--xsmall)", background: "transparent", color: "var(--white)" }} | ||
| /> */} | ||
| /> | ||
|
|
||
| <FormInput readOnly type="textarea" rows={20} value={submissionData?.submissionsDescriptions?.submissionMessage} /> | ||
| {/* <FormInput readOnly type="textarea" rows={20} value={submissionData?.submissionsDescriptions?.submissionMessage} /> */} | ||
|
|
||
| <div className="buttons"> | ||
| {submissionData.ref === "audit-wizard" && ( | ||
| <Button | ||
| filledColor="secondary" | ||
| disabled={isSubmitting || isSigningSubmission} | ||
| bigHorizontalPadding | ||
| onClick={() => submitSubmission()} | ||
| > | ||
| {t("Submissions.editSubmission")} | ||
| </Button> | ||
| )} |
There was a problem hiding this comment.
The new code introduces a Markdown editor for displaying the submission message. This is a good improvement as it allows the user to view the message in a more readable format. However, there's a potential issue with the MDEditor.Markdown component. The disallowedElements prop is being used but it's not clear if this prop actually exists on the MDEditor.Markdown component based on the documentation available up until my knowledge cutoff in September 2021. Please verify that this prop is valid and works as expected.
Also, the button for editing the submission is only shown if submissionData.ref === "audit-wizard". If this condition is not met, the user won't be able to edit their submission. If this is the intended behavior, then it's fine. Otherwise, you might want to revise this logic.
<MDEditor.Markdown
-42: disallowedElements={disallowedElementsMarkdown}
source={submissionData?.submissionsDescriptions?.submissionMessage}
style={{ whiteSpace: "normal", fontSize: "var(--xsmall)", background: "transparent", color: "var(--white)" }}
/>
{/* <FormInput readOnly type="textarea" rows={20} value={submissionData?.submissionsDescriptions?.submissionMessage} /> */}
<div className="buttons">
{submissionData.ref === "audit-wizard" && (
<Button
filledColor="secondary"
disabled={isSubmitting || isSigningSubmission}
bigHorizontalPadding
onClick={() => submitSubmission()}
>
{t("Submissions.editSubmission")}
</Button
)}| const auditWizardExample = JSON.parse(`{ | ||
| "project": { | ||
| "projectId": "0x18fbe473b99b3d68f5ad35881149ea0e1b56e091" | ||
| }, | ||
| "signature": "15bcdbe3bf773174702edd02067437d59d8f3f963cebd8661a83baf89cb75bc4d4e55653e7a1f0357f5532e9ea40b4eb78a4cbbe9bc51130505b64976fa1238de8920bc23312a0c5f9f3a772ce29cfd2ad39a9d18572173621fbc99cb7a31ca9538ff468e0ab743ba202667fcd24870e5d0d14eb6a8a0adfb5d23db208e427dd83ab757e4cf0d0dc8da215f18bb2587d94cfa94e40bf7f8e03dee0e02bc5a14bd3829b9d8d25884397b432aef10aa361431d8f59fc5a97e5a129e507509cdf36b1052d7d432c0701c5dfbe92ef532bf6c38a340c98a05b53a8856ff1f3afd97297815a1b82ae0cd2e71166706232a03eb9fc24c1db70ae273632fc658b28ab61", | ||
| "contact": { | ||
| "beneficiary": "0x56E889664F5961452E5f4183AA13AF568198eaD2", | ||
| "communicationChannel": "[email protected]", | ||
| "communicationChannelType": "email" | ||
| }, | ||
| "submissionDescriptions": { | ||
| "descriptions": [ | ||
| { | ||
| "title": "Malicious pair can re-enter 'VeryFastRouter' to drain funds", | ||
| "description": "VeryFastRouter::swap is the main entry point for a user to perform a batch of sell and buy orders on the new Sudoswap router, allowing partial fill conditions to be specified. Sell orders are executed first, followed by buy orders. The LSSVMPair contracts themselves are implemented in such a way that re-entrancy is not possible, but the same is not true of the VeryFastRouter. Assuming a user calls VeryFastRouter::swap, selling some NFTs and passing in some additional ETH value for subsequent buy orders, an attacker can re-enter this function under certain conditions to steal the original caller's funds. Given that this function does not check whether the user input contains valid pairs, an attacker can use this to manipulate", | ||
| "severity": "high severity", | ||
| "files": ["base64", "base64"] | ||
| }, | ||
| { | ||
| "title": "Lack of Input Validation in Transfer Function", | ||
| "description": "The smart contract being audited exhibits a low severity issue related to the lack of input validation in the transfer function. The contract allows users to transfer tokens between addresses, but it fails to adequately validate the input parameters, which can lead to potential vulnerabilities.", | ||
| "severity": "Low", | ||
| "files": ["base64"] | ||
| } | ||
| ] | ||
| } | ||
| }`); | ||
|
|
||
| export const SubmissionFormPage = () => { | ||
| const { t } = useTranslation(); | ||
| const confirm = useConfirm(); | ||
|
|
||
| const [searchParams] = useSearchParams(); | ||
| const { chain } = useNetwork(); | ||
| const [currentStep, setCurrentStep] = useState<number>(); | ||
| const [submissionData, setSubmissionData] = useState<ISubmissionData>(); | ||
| const [allFormDisabled, setAllFormDisabled] = useState(false); | ||
|
|
||
| const { activeVaults } = useVaults(); | ||
| const { activeVaults, vaultsReadyAllChains } = useVaults(); |
There was a problem hiding this comment.
The auditWizardExample object is hardcoded and seems to be used for testing purposes. It's not a good practice to leave test data in production code. If this is meant for testing, consider moving it to a separate test file or environment.
- const auditWizardExample = JSON.parse(`{...}`);| const handleClearSubmission = async () => { | ||
| const wantsToClear = await confirm({ | ||
| title: t("clearSubmission"), | ||
| titleIcon: <ClearIcon className="mr-2" fontSize="large" />, | ||
| description: t("clearSubmissionExplanation"), | ||
| cancelText: t("no"), | ||
| confirmText: t("clearForm"), | ||
| }); | ||
|
|
||
| if (!wantsToClear) return; | ||
| reset(); | ||
| }; | ||
|
|
||
| const populateDataFromAuditWizard = async (auditWizardSubmission: any) => { | ||
| if (!vaultsReadyAllChains) return; | ||
|
|
||
| if (submissionData?.project?.projectId) { | ||
| const wantsToClear = await confirm({ | ||
| title: t("existingSubmission"), | ||
| titleIcon: <ClearIcon className="mr-2" fontSize="large" />, | ||
| description: t("clearExistingSubmissionAuditWizardExplanation"), | ||
| cancelText: t("no"), | ||
| confirmText: t("clearForm"), | ||
| }); | ||
| if (!wantsToClear) return; | ||
| } | ||
|
|
||
| reset(); | ||
|
|
||
| const vault = activeVaults && activeVaults.find((vault) => vault.id === auditWizardSubmission.project.projectId); | ||
| if (!vault || !vault.description) { | ||
| return confirm({ | ||
| title: t("projectNotAvailable"), | ||
| titleIcon: <ErrorIcon className="mr-2" fontSize="large" />, | ||
| description: t("projectNotAvailableExplanation"), | ||
| confirmText: t("gotIt"), | ||
| }); | ||
| } | ||
|
|
||
| setSubmissionData((prev) => ({ | ||
| ...prev!, | ||
| ref: "audit-wizard", | ||
| project: { | ||
| verified: true, | ||
| projectName: vault.description?.["project-metadata"].name!, | ||
| ...auditWizardSubmission.project, | ||
| }, | ||
| terms: { verified: false }, | ||
| contact: { verified: false, ...auditWizardSubmission.contact }, | ||
| submissionsDescriptions: { | ||
| verified: false, | ||
| submission: "", | ||
| submissionMessage: "", | ||
| descriptions: auditWizardSubmission.submissionDescriptions.descriptions.map((desc: any) => { | ||
| const severity = (vault.description?.severities as IVulnerabilitySeverity[]).find( | ||
| (sev) => | ||
| desc.severity.toLowerCase()?.includes(sev.name.toLowerCase()) || | ||
| sev.name.toLowerCase()?.includes(desc.severity.toLowerCase()) | ||
| ); | ||
| return { | ||
| title: desc.title, | ||
| description: desc.description, | ||
| severity: severity?.name.toLowerCase() ?? desc.severity.toLowerCase(), | ||
| isEncrypted: !severity?.decryptSubmissions, | ||
| files: [], | ||
| }; | ||
| }), | ||
| }, | ||
| submissionResult: undefined, | ||
| })); | ||
| setAllFormDisabled(true); | ||
| }; |
There was a problem hiding this comment.
In the populateDataFromAuditWizard function, you're using any as the type of auditWizardSubmission. This is not recommended because it bypasses the type checking. Consider creating an interface that describes the shape of auditWizardSubmission.
- const populateDataFromAuditWizard = async (auditWizardSubmission: any) => {
+ interface IAuditWizardSubmission {
+ // define the properties here
+ }
+
+ const populateDataFromAuditWizard = async (auditWizardSubmission: IAuditWizardSubmission) => {| allFormDisabled, | ||
| }; | ||
|
|
||
| return ( | ||
| <> | ||
| <Button | ||
| className="mb-4" | ||
| styleType="invisible" | ||
| textColor="error" | ||
| onClick={() => populateDataFromAuditWizard(auditWizardExample)} | ||
| > | ||
| <ClearIcon className="mr-2" /> | ||
| {"TEST"} | ||
| </Button> | ||
| <Seo title={t("seo.submitVulnerabilityTitle")} /> | ||
| <StyledSubmissionFormPage className="content-wrapper"> | ||
| <div className="top-controls"> | ||
| {submissionData?.ref === "audit-wizard" && ( | ||
| <div className="auditWizardSubmission"> | ||
| <img src={require("assets/images/audit_wizard.png")} alt="audit wizard logo" /> | ||
| <p>{t("Submissions.submissionSubmittedViaAuditWizard")}</p> | ||
| </div> | ||
| )} | ||
| {submissionData?.project?.projectId && ( | ||
| <Button styleType="invisible" textColor="error" onClick={handleClearSubmission}> | ||
| <ClearIcon className="mr-2" /> | ||
| {t("clearSubmission")} | ||
| </Button> | ||
| )} | ||
| </div> | ||
|
|
There was a problem hiding this comment.
The button with label "TEST" seems to be used for testing purposes. It's not a good practice to leave test buttons or any other test elements in production code. If this is meant for testing, consider removing it from the production code.
- <Button
- className="mb-4"
- styleType="invisible"
- textColor="error"
- onClick={() => populateDataFromAuditWizard(auditWizardExample)}
- >
- <ClearIcon className="mr-2" />
- {"TEST"}
- </Button>
fonstack
changed the base branch from
develop
to
feature/auditwizard-integration
There was a problem hiding this comment.
Files selected (8)
- packages/web/src/constants/constants.ts (1)
- packages/web/src/pages/CommitteeTools/SubmissionsTool/SubmissionDetailsPage/SubmissionDetailsPage.tsx (2)
- packages/web/src/pages/Honeypots/VaultDetailsPage/Sections/VaultScopeSection/EnvSetupSection/EnvSetupSection.tsx (2)
- packages/web/src/pages/Honeypots/VaultDetailsPage/Sections/VaultScopeSection/InScopeSection/InScopeSection.tsx (4)
- packages/web/src/pages/Honeypots/VaultDetailsPage/Sections/VaultScopeSection/OutOfScopeSection/OutOfScopeSection.tsx (2)
- packages/web/src/pages/Honeypots/VaultDetailsPage/Sections/VaultScopeSection/SeverityLevelsSection/SeverityLevelsSection.tsx (2)
- packages/web/src/pages/Honeypots/VaultDetailsPage/Sections/VaultSubmissionsSection/PublicSubmissionCard/PublicSubmissionCard.tsx (1)
- packages/web/src/pages/Submissions/SubmissionFormPage/FormSteps/SubmissionSubmit/components/SubmissionReview/SubmissionReview.tsx (2)
Files ignored due to filter (1)
- packages/web/src/languages/en.json
Review comments generated (18)
- Review: 17
- LGTM: 1
Tips
Chat with CodeRabbit Bot (@coderabbitai)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitaiin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
| export const allowedElementsMarkdown = [ | ||
| "h1", | ||
| "h2", | ||
| "h3", | ||
| "h4", | ||
| "h5", | ||
| "h6", | ||
| "p", | ||
| "a", | ||
| "ul", | ||
| "ol", | ||
| "li", | ||
| "strong", | ||
| "em", | ||
| "s", | ||
| "code", | ||
| "blockquote", | ||
| "pre", | ||
| "img", | ||
| "hr", | ||
| "br", | ||
| "table", | ||
| "thead", | ||
| "tbody", | ||
| "tr", | ||
| "th", | ||
| "td", | ||
| "caption", | ||
| "div", | ||
| "span", | ||
| ]; |
There was a problem hiding this comment.
The change from disallowedElementsMarkdown to allowedElementsMarkdown is a good security practice. It's better to have an allowlist than a blocklist, as it's impossible to predict all possible harmful elements that could be used in Markdown. However, I noticed that you're allowing the <img> tag in your Markdown. This can potentially lead to security issues such as XSS (Cross-Site Scripting) attacks if not properly sanitized. You should consider removing it from the allowed list or ensure that the source of images is safe and trusted.
- 143: "img",| @@ -1,7 +1,7 @@ | |||
| import { IVault, IVulnerabilitySeverity } from "@hats-finance/shared"; | |||
| import MDEditor from "@uiw/react-md-editor"; | |||
| import { Pill } from "components"; | |||
| import { disallowedElementsMarkdown } from "constants/constants"; | |||
| import { allowedElementsMarkdown } from "constants/constants"; | |||
There was a problem hiding this comment.
The import statement has been changed from disallowedElementsMarkdown to allowedElementsMarkdown. This change seems to be consistent with the changes made in other parts of the code. However, please ensure that the allowedElementsMarkdown array contains only safe HTML elements to prevent potential XSS attacks.
- import { disallowedElementsMarkdown } from "constants/constants";
+ import { allowedElementsMarkdown } from "constants/constants";| @@ -20,7 +20,7 @@ export const SeverityLevelsSection = ({ vault }: SeverityLevelsSectionProps) => | |||
| <div className="severity" key={idx}> | |||
| <Pill isSeverity transparent text={severity.name} textColor={severityColors[idx]} /> | |||
| <MDEditor.Markdown | |||
| disallowedElements={disallowedElementsMarkdown} | |||
| allowedElements={allowedElementsMarkdown} | |||
There was a problem hiding this comment.
The prop name for the MDEditor.Markdown component has been updated from disallowedElements to allowedElements. This change is consistent with the updated import statement. Again, make sure that the allowedElementsMarkdown array only includes safe HTML elements.
- disallowedElements={disallowedElementsMarkdown}
+ allowedElements={allowedElementsMarkdown}| @@ -1,7 +1,7 @@ | |||
| import { IVault } from "@hats-finance/shared"; | |||
| import MDEditor from "@uiw/react-md-editor"; | |||
| import { Pill } from "components"; | |||
| import { disallowedElementsMarkdown } from "constants/constants"; | |||
| import { allowedElementsMarkdown } from "constants/constants"; | |||
There was a problem hiding this comment.
The import statement has been changed from disallowedElementsMarkdown to allowedElementsMarkdown. This change is significant as it alters the elements that are allowed in the Markdown rendering. Ensure that this change does not introduce any security vulnerabilities by allowing potentially harmful HTML elements.
- import { disallowedElementsMarkdown } from "constants/constants";
+ import { allowedElementsMarkdown } from "constants/constants";| @@ -20,7 +20,7 @@ export const EnvSetupSection = ({ vault }: EnvSetupSectionProps) => { | |||
| </div> | |||
|
|
|||
| <MDEditor.Markdown | |||
| disallowedElements={disallowedElementsMarkdown} | |||
| allowedElements={allowedElementsMarkdown} | |||
There was a problem hiding this comment.
The prop name for the MDEditor.Markdown component has been updated from disallowedElements to allowedElements. This change aligns with the updated import statement and affects which HTML elements are allowed in the Markdown rendering. Again, ensure that this change does not introduce any security vulnerabilities by allowing potentially harmful HTML elements.
- disallowedElements={disallowedElementsMarkdown}
+ allowedElements={allowedElementsMarkdown}| <Alert type="info" className="mb-4"> | ||
| {t("loadingContractsOnRepo")} | ||
| </Alert> |
There was a problem hiding this comment.
The alert component now includes a className prop and the message has been moved to a new line. This should improve readability and maintainability of the code.
- <Alert type="info">{t("loadingContractsOnRepo")}</Alert>
+ <Alert type="info" className="mb-4">
+ {t("loadingContractsOnRepo")}
+ </Alert>| @@ -1,6 +1,6 @@ | |||
| import MDEditor from "@uiw/react-md-editor"; | |||
| import { Alert, Button, FormInput, Loading } from "components"; | |||
| import { disallowedElementsMarkdown } from "constants/constants"; | |||
| import { allowedElementsMarkdown } from "constants/constants"; | |||
There was a problem hiding this comment.
The import statement has been changed from disallowedElementsMarkdown to allowedElementsMarkdown. This change will affect the way Markdown is rendered. Please ensure that all elements in allowedElementsMarkdown are safe to render and do not pose any security risks, such as XSS (Cross-Site Scripting).
- import { disallowedElementsMarkdown } from "constants/constants";
+ import { allowedElementsMarkdown } from "constants/constants";| @@ -39,7 +39,7 @@ export function SubmissionReview() { | |||
| <br /> | |||
|
|
|||
| <MDEditor.Markdown | |||
| disallowedElements={disallowedElementsMarkdown} | |||
| allowedElements={allowedElementsMarkdown} | |||
There was a problem hiding this comment.
The prop name for the MDEditor.Markdown component has been changed from disallowedElements to allowedElements. This change will affect the way Markdown is rendered. Please ensure that all elements in allowedElementsMarkdown are safe to render and do not pose any security risks, such as XSS (Cross-Site Scripting).
- disallowedElements={disallowedElementsMarkdown}
+ allowedElements={allowedElementsMarkdown}| const [isOpen, setIsOpen] = useState(false); | ||
|
|
||
| const severityColors = getSeveritiesColorsArray(vault); | ||
| const severityIndex = | ||
| submission.severity && | ||
| vault?.description?.severities.findIndex((sev: IVulnerabilitySeverity) => | ||
| sev.name.toLowerCase().includes(submission.severity ?? "") | ||
| ); |
There was a problem hiding this comment.
The code is trying to find the index of a severity level in the vault.description.severities array that matches the submission.severity. However, it's using toLowerCase() on the severity name but not on the submission.severity, which could lead to inconsistencies if the case doesn't match. I suggest converting both to lowercase for comparison.
- sev.name.toLowerCase().includes(submission.severity ?? "")
+ sev.name.toLowerCase().includes((submission.severity ?? "").toLowerCase())| <StyledPublicSubmissionCard isOpen={isOpen}> | ||
| <div className="card-header" onClick={() => setIsOpen((prev) => !prev)}> | ||
| <div className="severity"> | ||
| <Pill textColor={severityColors[severityIndex ?? 0]} isSeverity text={submission.severity ?? t("noSeverity")} /> |
There was a problem hiding this comment.
The code is defaulting to the first color in severityColors when severityIndex is undefined. This might be misleading if the first color does not represent "no severity". Consider adding a neutral color for "no severity" cases.
- <Pill textColor={severityColors[severityIndex ?? 0]} isSeverity text={submission.severity ?? t("noSeverity")} />
+ <Pill textColor={severityIndex !== undefined ? severityColors[severityIndex] : 'neutralColor'} isSeverity text={submission.severity ?? t("noSeverity")} />
Summary by CodeRabbit
VaultSubmissionsSectionto theVaultDetailsPage, displaying public submissions related to a vault and providing a direct link to the corresponding GitHub issues page.VaultDetailsPage.SubmissionFormPagewith a new state variableallFormDisabledto control form inputs, improving user experience during data entry.allowedElementsMarkdown, enhancing content security.contractsCovered.utils.tsby leveraging sessionStorage, reducing unnecessary server requests.submitVulnerabilitySubmissionfunction for better error handling when accessing potentially undefined properties.