Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feat] Audit wizard integration #552

Merged
merged 11 commits into from
Sep 18, 2023
Binary file added packages/web/src/assets/images/audit_wizard.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 2 additions & 0 deletions packages/web/src/constants/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ export const MAX_NFT_TIER = 3;
export const stagingServiceUrl = "https://hats-backend-dev.herokuapp.com/v1";
export const prodServiceUrl = "https://hats-backend-prod.herokuapp.com/v1";

export const auditWizardVerifyService = "https://app.auditwizard.io:5000/submissions/finalizeHats";

Choose a reason for hiding this comment

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

Image description CodeRabbit

The URL for the auditWizardVerifyService is hardcoded. This could lead to issues when you need to switch between different environments (like staging, production, etc.). Consider using environment variables for such values.

- export const auditWizardVerifyService = "https://app.auditwizard.io:5000/submissions/finalizeHats";
+ export const auditWizardVerifyService = process.env.AUDIT_WIZARD_VERIFY_SERVICE || "https://app.auditwizard.io:5000/submissions/finalizeHats";

This way, you can set the AUDIT_WIZARD_VERIFY_SERVICE environment variable as needed per environment, and if it's not set, it will default to the provided URL.


export const NFTContractDataProxy = {
["0xCCaadc293FaAEa229e0ca4A22B0330b65634b483".toLowerCase()]: "0x1d25bf3d0f8997282055a1a242fa2b146e7b4ec5",
["0x571f39d351513146248AcafA9D0509319A327C4D".toLowerCase()]: "0xe127be2bc276142039bc16251bb04e15b2b34f25",
Expand Down
16 changes: 15 additions & 1 deletion packages/web/src/languages/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@
"noOptions": "No options",
"success": "Success",
"failed": "Failed",
"peding": "Pending",
"pending": "Pending",
"finishingVaultCreation": "Finishing vault creation",
"weAreFinishingTheCreationOfVault": " Thanks for creating a new vault in Hats! \n\n We are finishing the creation of your vault. We need a couple more minutes. \n\n We will notify you via email you when the vault is created.",
"executeTxOnGnosisForCreatingVault": " Thanks for creating a new vault in Hats! \n\n You just create the proposal for creating the vault in Safe app. Please approve and execute the transaction on the Safe app. \n\n We will notify you via email you when the vault is created.",
Expand Down Expand Up @@ -582,6 +582,18 @@
"goToProjectWebsite": "Go to project website",
"doYouWantToGoToProjectWebsite": "Do you want to go to project website? \n ({{website}})",
"yesGo": "Yes, go",
"clearSubmission": "Clear submission",
"clearSubmissionExplanation": "Are you sure you want to clear this submission? \n\n This will remove all the information of the submission.",
"clearForm": "Clear form",
"existingSubmission": "Existing submission",
"clearExistingSubmissionAuditWizardExplanation": "Attention, you have started to create a submission, in order to submit a new one from AuditWizard you need to remove the current one. \n\nAre you sure you want to clear this submission? \n This will remove all the information of the submission.",
"projectNotAvailable": "Project not available",
"projectNotAvailableExplanation": "The project on which you are attempting to submit a vulnerability is no longer available. \n\n If you think this is an error, please contact Hats support.",
"understand": "Understand",
"submissionChanged": "Submission changed",
"submissionChangedExplanationAuditWizard": "The submission you are trying to edit has changed. \n\n If you want to edit the submission, please go back to Audit Wizard.",
"submissionNotValid": "Submission not valid",
"submissionNotValidExplanationAuditWizard": "The submission you are trying to edit is not valid. \n\n Please go back to Audit Wizard and try again.",
"PGPTool": {
"title": "PGP tool",
"unlockPgpTool": "Unlock the PGP tool",
Expand Down Expand Up @@ -829,6 +841,8 @@
"encryptedSubmission": "Encrypted submission",
"encryptedSubmissionExplanation": "The submission is encrypted and only the committee can decrypt it.",
"decryptedSubmissionExplanation": "The submission is decrypted and public. This normally happens on Audit Competitions.",
"submissionSubmittedViaAuditWizard": "Submission submitted via auditwizard.io",
"editSubmission": "Edit submission",
"terms": {
"bugBounty": {
"submissionSection": "<p>Your submission will be sent to the committee who will process your submission.</p> <p>The committee of the vault will respond within 12 hours to acknowledge the receipt of submission via the communication channel you provided.<p/>",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { StyledContactInfo } from "./styles";
export function SubmissionContactInfo() {
const { t } = useTranslation();
const { address: account } = useAccount();
const { submissionData, setSubmissionData, vault } = useContext(SubmissionFormContext);
const { submissionData, setSubmissionData, vault, allFormDisabled } = useContext(SubmissionFormContext);

Choose a reason for hiding this comment

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

Image description CodeRabbit

The new variable allFormDisabled is introduced via the useContext hook. This variable is used to disable certain form inputs and buttons based on its value. It's a good practice to control the state of the form elements based on certain conditions.

-  const { submissionData, setSubmissionData, vault } = useContext(SubmissionFormContext);
+  const { submissionData, setSubmissionData, vault, allFormDisabled } = useContext(SubmissionFormContext);


const isAuditCompetition = vault?.description?.["project-metadata"].type === "audit";

Expand Down Expand Up @@ -44,7 +44,7 @@ export function SubmissionContactInfo() {
setSubmissionData((prev) => {
if (prev)
return {
...prev!,
...prev,

Choose a reason for hiding this comment

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

Image description CodeRabbit

The spread operator (...) is used to create a shallow copy of the prev object. This is a good practice when you want to update the state in React because it creates a new object with the same properties, which helps React optimize re-rendering.

-          ...prev!,
+          ...prev,
           contact: { ...contactData, verified: true },
           submissionsDescriptions: { ...prev.submissionsDescriptions, verified: false },
           submissionResult: undefined,

contact: { ...contactData, verified: true },
submissionsDescriptions: { ...prev.submissionsDescriptions, verified: false },
submissionResult: undefined,
Expand All @@ -70,13 +70,15 @@ export function SubmissionContactInfo() {

<FormInput
{...register("beneficiary")}
disabled={allFormDisabled}

Choose a reason for hiding this comment

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

Image description CodeRabbit

The disabled attribute is added to the FormInput component and its value is set to allFormDisabled. This will disable the input field if allFormDisabled is true.

-        {...register("beneficiary")}
+        {...register("beneficiary")}
+        disabled={allFormDisabled}
         label={`${t("beneficiaryWalletAddress")}`}
         placeholder={t("beneficiaryWalletAddressPlaceholder")}
         colorable

label={`${t("beneficiaryWalletAddress")}`}
placeholder={t("beneficiaryWalletAddressPlaceholder")}
colorable
/>

<FormRadioInput
{...register("communicationChannelType")}
disabled={allFormDisabled}

Choose a reason for hiding this comment

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

Image description CodeRabbit

The disabled attribute is added to the FormRadioInput component and its value is set to allFormDisabled. This will disable the radio input if allFormDisabled is true.

-        {...register("communicationChannelType")}
+        {...register("communicationChannelType")}
+        disabled={allFormDisabled}
         label={t("Submissions.addPreferredCommunicationChannel")}
         radioOptions={[
           { label: t("discord"), value: "discord" },

label={t("Submissions.addPreferredCommunicationChannel")}
radioOptions={[
{ label: t("discord"), value: "discord" },
Expand All @@ -87,6 +89,7 @@ export function SubmissionContactInfo() {

<FormInput
{...register("communicationChannel")}
disabled={allFormDisabled}

Choose a reason for hiding this comment

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

Image description CodeRabbit

The disabled attribute is added to the FormInput component and its value is set to allFormDisabled. This will disable the input field if allFormDisabled is true.

-        {...register("communicationChannel")}
+        {...register("communicationChannel")}
+        disabled={allFormDisabled}
         label={communicationChannelLabel}
         placeholder={t("enterCommunicationChannel")}
         colorable

label={communicationChannelLabel}
placeholder={t("enterCommunicationChannel")}
colorable
Expand All @@ -97,6 +100,7 @@ export function SubmissionContactInfo() {
<p className="mb-2">{t("Submissions.addGithubAccountConnectIssue")}</p>
<FormInput
{...register("githubUsername")}
disabled={allFormDisabled}

Choose a reason for hiding this comment

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

Image description CodeRabbit

The disabled attribute is added to the FormInput component and its value is set to allFormDisabled. This will disable the input field if allFormDisabled is true.

-            {...register("githubUsername")}
+            {...register("githubUsername")}
+            disabled={allFormDisabled}
             label={`${t("githubUsername")}`}
             placeholder={t("githubUsernamePlaceholder")}
             colorable

label={`${t("githubUsername")}`}
placeholder={t("githubUsernamePlaceholder")}
colorable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
const [severitiesOptions, setSeveritiesOptions] = useState<FormSelectInputOption[] | undefined>();

const isAuditSubmission = vault?.description?.["project-metadata"].type === "audit";
Expand Down Expand Up @@ -124,6 +124,7 @@ export function SubmissionDescriptions() {

const { encryptedData, sessionKey } = encryptionResult;
const submissionInfo = {
ref: submissionData.ref,
decrypted,
encrypted: encryptedData as string,
};
Expand Down Expand Up @@ -178,6 +179,7 @@ export function SubmissionDescriptions() {
<div className="row">
<FormInput
{...register(`descriptions.${index}.title`)}
disabled={allFormDisabled}
label={`${t("Submissions.submissionTitle")}`}
placeholder={t("Submissions.submissionTitlePlaceholder")}
colorable
Expand All @@ -187,6 +189,7 @@ export function SubmissionDescriptions() {
name={`descriptions.${index}.severity`}
render={({ field, fieldState: { error }, formState: { dirtyFields, defaultValues } }) => (
<FormSelectInput
disabled={allFormDisabled}
isDirty={getCustomIsDirty<ISubmissionsDescriptionsData>(field.name, dirtyFields, defaultValues)}
error={error}
label={t("severity")}
Expand All @@ -204,6 +207,7 @@ export function SubmissionDescriptions() {
name={`descriptions.${index}.description`}
render={({ field, fieldState: { error }, formState: { dirtyFields, defaultValues } }) => (
<FormMDEditor
disabled={allFormDisabled}
isDirty={getCustomIsDirty<ISubmissionsDescriptionsData>(field.name, dirtyFields, defaultValues)}
error={error}
colorable
Expand All @@ -212,7 +216,7 @@ export function SubmissionDescriptions() {
)}
/>

{!submissionDescription.isEncrypted && (
{!submissionDescription.isEncrypted && !allFormDisabled && (
<Controller
control={control}
name={`descriptions.${index}.files`}
Expand All @@ -222,7 +226,7 @@ export function SubmissionDescriptions() {
/>
)}

{controlledDescriptions.length > 1 && (
{controlledDescriptions.length > 1 && !allFormDisabled && (
<div className="buttons mt-3">
<Button onClick={() => removeSubmissionDescription(index)} styleType="outlined" filledColor="secondary">
<RemoveIcon className="mr-3" />
Expand All @@ -235,13 +239,15 @@ export function SubmissionDescriptions() {
})}

<div className="buttons mt-3">
<Button
onClick={() => appendSubmissionDescription(SUBMISSION_INIT_DATA.submissionsDescriptions.descriptions[0])}
styleType="invisible"
>
<AddIcon className="mr-3" />
{t("Submissions.addAnotherVulnerability")}
</Button>
{!allFormDisabled && (
<Button
onClick={() => appendSubmissionDescription(SUBMISSION_INIT_DATA.submissionsDescriptions.descriptions[0])}
styleType="invisible"
>
<AddIcon className="mr-3" />
{t("Submissions.addAnotherVulnerability")}
</Button>
)}
<Button onClick={handleSubmit(handleSaveAndDownloadDescription)}>{t("Submissions.saveAndDownload")}</Button>
</div>
</StyledSubmissionDescriptionsList>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export const getGithubIssueDescription = (
submissionData: ISubmissionData,
description: ISubmissionsDescriptionsData["descriptions"][0]
) => {
return `
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}
Comment on lines +91 to 94

Choose a reason for hiding this comment

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

Image description CodeRabbit

The changes made here are adding a conditional check to modify the generated GitHub issue description based on whether the submission is from the audit wizard or not. This seems like a reasonable change, but it would be better if the string "audit-wizard" was replaced with a constant to avoid potential typos and improve maintainability.

- return `${submissionData.ref === "audit-wizard" ? "***Submitted via auditwizard.io***\n" : ""}`
+ const AUDIT_WIZARD_REF = "audit-wizard";
+ return `${submissionData.ref === AUDIT_WIZARD_REF ? "***Submitted via auditwizard.io***\n" : ""}`

This way, if the reference for the audit wizard ever needs to be changed, it can be done in one place rather than having to find all instances of the string in the codebase.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { StyledSubmissionProject } from "./styles";

export function SubmissionProject() {
const { t } = useTranslation();
const { submissionData, setSubmissionData } = useContext(SubmissionFormContext);
const { submissionData, setSubmissionData, allFormDisabled } = useContext(SubmissionFormContext);

Choose a reason for hiding this comment

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

Image description CodeRabbit

The new variable allFormDisabled is introduced from the context. It's good to see that you're using context to manage global state, but it would be better if you could add some error handling here. If for some reason the context does not provide this variable, it could lead to unexpected behavior or crashes. Consider providing a default value when destructuring the context.

- const { submissionData, setSubmissionData, allFormDisabled } = useContext(SubmissionFormContext);
+ const { submissionData, setSubmissionData, allFormDisabled = false } = useContext(SubmissionFormContext);

const [userInput, setUserInput] = useState("");
const { activeVaults } = useVaults();

Expand All @@ -36,7 +36,7 @@ export function SubmissionProject() {
key={index}
vault={vault}
expanded={false}
onSelect={() => handleSelectedProject(vault)}
onSelect={!allFormDisabled ? () => handleSelectedProject(vault) : () => {}}

Choose a reason for hiding this comment

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

Image description CodeRabbit

The ternary operator used here is a bit confusing and might lead to code readability issues. Instead of using an empty function as a fallback, consider moving the condition inside the handleSelectedProject function. This way, the onSelect prop always points to the same function, which internally decides whether to perform any action based on the allFormDisabled flag.

- onSelect={!allFormDisabled ? () => handleSelectedProject(vault) : () => {}}
+ onSelect={() => handleSelectedProject(vault)}

And then in your handleSelectedProject function:

- const handleSelectedProject = (vault) => {
-   // existing logic...
- }
+ const handleSelectedProject = (vault) => {
+   if (allFormDisabled) return;
+   // existing logic...
+ }

selected={submissionData?.project?.projectId === vault.id}
/>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export function SubmissionProcessed() {

const getSubmissionOnServerStatus = () => {
let color = Colors.yellow;
let text = t("pending");
let text = `${t("pending")}...`;

switch (submissionStatus.server) {
case SubmissionOpStatus.Success:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import MDEditor from "@uiw/react-md-editor";
import { Alert, Button, FormInput, Loading } from "components";
import { disallowedElementsMarkdown } from "constants/constants";
import { SubmissionFormContext } from "pages/Submissions/SubmissionFormPage/store";
import { useContext } from "react";
import { useTranslation } from "react-i18next";
Comment on lines +1 to 6

Choose a reason for hiding this comment

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

Image description CodeRabbit

Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [1-7]

The import of MDEditor and disallowedElementsMarkdown is new in this hunk. Ensure that these are used appropriately in the code and that they do not introduce any security or performance issues. Also, make sure that the imported MDEditor component is accessible and user-friendly.

+ import MDEditor from "@uiw/react-md-editor";
+ import { disallowedElementsMarkdown } from "constants/constants";

Expand Down Expand Up @@ -34,15 +36,27 @@ export function SubmissionReview() {
/>
</div>

{/* <MDEditor.Markdown
<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>
)} */}
Comment on lines +39 to +59

Choose a reason for hiding this comment

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

Image description CodeRabbit

The addition of the MDEditor.Markdown component is a good change for rendering markdown content. However, it's important to sanitize the input to prevent Cross-Site Scripting (XSS) attacks. If the submissionData?.submissionsDescriptions?.submissionMessage can be controlled by users, ensure that it is sanitized before being passed to the source prop.

Also, there's commented-out code here. It's generally best to remove such code to keep the codebase clean and readable. If this code is needed for future reference, consider adding a descriptive comment explaining its purpose.

Lastly, the button for editing submissions when submissionData.ref === "audit-wizard" is commented out. If this functionality is not needed, consider removing the related code to avoid confusion.

+ <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} /> */}
- 
- <div className="buttons">
-   {/* {submissionData.ref === "audit-wizard" && (
-     <Button
-       filledColor="secondary"
-       disabled={isSubmitting || isSigningSubmission}
-       bigHorizontalPadding
-       onClick={() => submitSubmission()}
-     >
-       {t("Submissions.editSubmission")}
-     </Button>
-   )} */}
+ <Button disabled={isSubmitting || isSigningSubmission} bigHorizontalPadding onClick={() => submitSubmission()}>
+   {t("submit")}
+ </Button>

<Button disabled={isSubmitting || isSigningSubmission} bigHorizontalPadding onClick={() => submitSubmission()}>
{t("submit")}
</Button>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ export const StyledSubmissionReview = styled.div`
}

.buttons {
margin-top: ${getSpacing(2)};
display: flex;
gap: ${getSpacing(4)};
justify-content: flex-end;
}
`;
Loading