ronantakizawa/codereview-bench huggingface dataset:
Repository: Infisical/infisical
File: backend/src/db/migrations/20250824173438_add-approval-secret-read-compat.ts (TypeScript)

Code before:
import { Knex } from "knex";

import { TableName } from "../schemas";

export async function up(knex: Knex): Promise<void> {
  if (!(await knex.schema.hasColumn(TableName.SecretApprovalPolicy, "secretReadAccessCompat"))) {
    await knex.schema.alterTable(TableName.SecretApprovalPolicy, (t) => {
      t.boolean("secretReadAccessCompat").nullable();
    });
  }
}

export async function down(knex: Knex): Promise<void> {
  if (await knex.schema.hasColumn(TableName.SecretApprovalPolicy, "secretReadAccessCompat")) {
    await knex.schema.alterTable(TableName.SecretApprovalPolicy, (t) => {
      t.dropColumn("secretReadAccessCompat");
    });
  }
}


Review comment (security):
maybe we can use a better name for the field? Current one is a bit tricky to parse. Maybe something like `shouldCheckSecretPermission`? 

Code after:
import { Knex } from "knex";

import { TableName } from "../schemas";

export async function up(knex: Knex): Promise<void> {
  if (!(await knex.schema.hasColumn(TableName.SecretApprovalPolicy, "shouldCheckSecretPermission"))) {
    await knex.schema.alterTable(TableName.SecretApprovalPolicy, (t) => {
      t.boolean("shouldCheckSecretPermission").nullable();
    });
  }
}

export async function down(knex: Knex): Promise<void> {
  if (await knex.schema.hasColumn(TableName.SecretApprovalPolicy, "shouldCheckSecretPermission")) {
    await knex.schema.alterTable(TableName.SecretApprovalPolicy, (t) => {
      t.dropColumn("shouldCheckSecretPermission");
    });
  }
}
Repository: Infisical/infisical
File: frontend/src/components/secret-syncs/forms/SecretSyncDestinationFields/ChecklySyncFields.tsx (TypeScript)

Code before:
              getOptionValue={(option) => option.id}
            />
          </FormControl>
        )}
      />

      <Controller
        name="destinationConfig.groupId"
        control={control}
        render={({ field: { value, onChange }, fieldState: { error } }) => (
          <FormControl
            isError={Boolean(error)}
            errorText={error?.message}
            label="Select a group"
            isOptional
            helperText="If provided, secrets will be scoped to a check group instead"
            tooltipClassName="max-w-md"
          >
            <FilterableSelect
              isLoading={isGroupsLoading && Boolean(connectionId)}
              isDisabled={!connectionId}
              isClearable
              value={groups.find((p) => p.id === value) ?? null}
              onChange={(option) => {
                const v = option as SingleValue<TChecklyAccount>;
                onChange(v?.id ?? undefined);
                setValue("destinationConfig.groupName", v?.name ?? "");
              }}
              options={groups}
              placeholder="Select a group..."
              getOptionLabel={(option) => option.name}
              getOptionValue={(option) => option.id}
            />
          </FormControl>
        )}
      />
    </>
  );
};


Review comment (suggestion):
for some reason when editing an existing sync the clear button doesn't work (at least visually - I think it's because it's being set to undefined instead of null?) - works fine when creating one
<img width="1716" height="1422" alt="CleanShot 2025-08-27 at 14 14 26@2x" src="https://github.com/user-attachments/assets/d7b3e4f1-51ee-4cb4-b03a-e115e6878cbd" />


Code after:
              getOptionValue={(option) => option.id}
            />
          </FormControl>
        )}
      />

      <Controller
        name="destinationConfig.groupId"
        control={control}
        render={({ field: { value, onChange }, fieldState: { error } }) => (
          <FormControl
            isError={Boolean(error)}
            errorText={error?.message}
            label="Select a group"
            isOptional
            helperText="If provided, secrets will be scoped to a check group instead"
            tooltipClassName="max-w-md"
          >
            <FilterableSelect
              isLoading={isGroupsLoading && Boolean(connectionId)}
              isDisabled={!connectionId}
              isClearable
              value={groups.find((p) => p.id === value) ?? null}
              onChange={(option) => {
                const v = option as SingleValue<TChecklyAccount>;
                onChange(v?.id ?? null);
                setValue("destinationConfig.groupName", v?.name ?? undefined);
              }}
              options={groups}
              placeholder="Select a group..."
              getOptionLabel={(option) => option.name}
              getOptionValue={(option) => option.id}
            />
          </FormControl>
        )}
      />
    </>
  );
};
Repository: Infisical/infisical
File: frontend/src/pages/organization/ProjectsPage/components/ProjectListToggle.tsx (TypeScript)

Code before:
import { Button } from "@app/components/v2";

export enum ProjectListView {
  MyProjects = "my-projects",
  AllProjects = "all-projects"
}

type Props = {
  value: ProjectListView;
  onChange: (value: ProjectListView) => void;
};

export const ProjectListToggle = ({ value, onChange }: Props) => {
  return (
    <div className="flex rounded-md border border-mineshaft-600 bg-mineshaft-800 p-1">
      <Button
        variant="outline_bg"
        onClick={() => {
          onChange(ProjectListView.MyProjects);
        }}
        size="xs"
        className={`${
          value === ProjectListView.MyProjects ? "bg-mineshaft-500" : "bg-transparent"
        } min-w-[2.4rem] rounded border-none hover:bg-mineshaft-600`}
      >
        My Projects
      </Button>
      <Button
        variant="outline_bg"
        onClick={() => {
          onChange(ProjectListView.AllProjects);
        }}
        size="xs"
        className={`${
          value === ProjectListView.AllProjects ? "bg-mineshaft-500" : "bg-transparent"
        } min-w-[2.4rem] rounded border-none hover:bg-mineshaft-600`}
      >
        All Projects
      </Button>
    </div>

Review comment (suggestion):
Let's try a gap-1 here to add some space between the buttons. Right now hovering looks weird

<img width="1024" height="281" alt="Image" src="https://github.com/user-attachments/assets/c5b0325b-0cab-49b8-9db7-95e3ddde0e8f" />

Code after:
import { Button } from "@app/components/v2";

export enum ProjectListView {
  MyProjects = "my-projects",
  AllProjects = "all-projects"
}

type Props = {
  value: ProjectListView;
  onChange: (value: ProjectListView) => void;
};

export const ProjectListToggle = ({ value, onChange }: Props) => {
  return (
    <div className="flex gap-x-0.5 rounded-md border border-mineshaft-600 bg-mineshaft-800 p-1">
      <Button
        variant="outline_bg"
        onClick={() => {
          onChange(ProjectListView.MyProjects);
        }}
        size="xs"
        className={`${
          value === ProjectListView.MyProjects ? "bg-mineshaft-500" : "bg-transparent"
        } min-w-[2.4rem] rounded border-none hover:bg-mineshaft-600`}
      >
        My Projects
      </Button>
      <Button
        variant="outline_bg"
        onClick={() => {
          onChange(ProjectListView.AllProjects);
        }}
        size="xs"
        className={`${
          value === ProjectListView.AllProjects ? "bg-mineshaft-500" : "bg-transparent"
        } min-w-[2.4rem] rounded border-none hover:bg-mineshaft-600`}
      >
        All Projects
      </Button>
    </div>
Repository: Infisical/infisical
File: backend/src/services/health-alert/health-alert-queue.ts (TypeScript)

Code before:
    }

    await queueService.stopRepeatableJob(
      QueueName.HealthAlert,
      QueueJobs.HealthAlert,
      { pattern: "*/5 * * * *", utc: true },
      QueueName.HealthAlert // job id
    );

    await queueService.startPg<QueueName.HealthAlert>(
      QueueJobs.HealthAlert,
      async () => {
        try {
          logger.info(`${QueueName.HealthAlert}: health check alert task started`);
          await gatewayV2Service.healthcheckNotify();
          await relayService.healthcheckNotify();
          logger.info(`${QueueName.HealthAlert}: health check alert task completed`);
        } catch (error) {
          logger.error(error, `${QueueName.HealthAlert}: health check alert failed`);
          throw error;
        }
      },
      {
        batchSize: 1,
        workerCount: 1,
        pollingIntervalSeconds: 1
      }
    );

    await queueService.schedulePg(
      QueueJobs.HealthAlert,
      "*/5 * * * *", // Schedule to run every 5 minutes
      undefined,
      { tz: "UTC" }
    );
  };

  return {
    init
  };
};


Review comment (suggestion):
Let's set the polling interval to a much higher number because there's no real-time expectation here. Probably 60 should be okay? 

For reference, the polling interval is what determines how often the queue polls our PG database for new jobs.. with the configuration, it will result in a READ query every second 

Code after:
    }

    await queueService.stopRepeatableJob(
      QueueName.HealthAlert,
      QueueJobs.HealthAlert,
      { pattern: "*/5 * * * *", utc: true },
      QueueName.HealthAlert // job id
    );

    await queueService.startPg<QueueName.HealthAlert>(
      QueueJobs.HealthAlert,
      async () => {
        try {
          logger.info(`${QueueName.HealthAlert}: health check alert task started`);
          await gatewayV2Service.healthcheckNotify();
          await relayService.healthcheckNotify();
          logger.info(`${QueueName.HealthAlert}: health check alert task completed`);
        } catch (error) {
          logger.error(error, `${QueueName.HealthAlert}: health check alert failed`);
          throw error;
        }
      },
      {
        batchSize: 1,
        workerCount: 1,
        pollingIntervalSeconds: 60
      }
    );

    await queueService.schedulePg(
      QueueJobs.HealthAlert,
      "*/5 * * * *", // Schedule to run every 5 minutes
      undefined,
      { tz: "UTC" }
    );
  };

  return {
    init
  };
};
Repository: Infisical/infisical
File: frontend/src/components/secret-syncs/forms/CreateSecretSyncForm.tsx (TypeScript)

Code before:
                        onCheckedChange={onChange}
                        isChecked={value}
                      >
                        <p className="w-[8.4rem]">Auto-Sync {value ? "Enabled" : "Disabled"}</p>
                      </Switch>
                    </FormControl>
                  );
                }}
              />
            </Tab.Panel>
            <Tab.Panel>
              <SecretSyncDetailsFields />
            </Tab.Panel>
            <Tab.Panel>
              <SecretSyncReviewFields />
            </Tab.Panel>
          </Tab.Panels>
        </Tab.Group>
      </FormProvider>

      <div className="flex w-full flex-row-reverse justify-between gap-4 pt-4">
        <Button
          onClick={handleNext}
          colorSchema="secondary"
          isDisabled={isCreateButtonDisabled || (isFinalStep && hasDuplicate)}
        >
          {isFinalStep ? "Create Sync" : "Next"}
        </Button>
        {selectedTabIndex > 0 && (
          <Button onClick={handlePrev} colorSchema="secondary">
            Back
          </Button>
        )}
      </div>
    </form>
  );
};


Review comment (suggestion):
I don't have block enabled but the create button is disabled

<img width="3456" height="1918" alt="CleanShot 2025-10-27 at 09 59 06@2x" src="https://github.com/user-attachments/assets/fc7f9f59-9342-426f-b43f-dfa78747784e" />
<img width="3456" height="1918" alt="CleanShot 2025-10-27 at 09 59 35@2x" src="https://github.com/user-attachments/assets/b70b1769-9a00-4df6-ac30-7bb03ec3d2ea" />


Code after:
                        onCheckedChange={onChange}
                        isChecked={value}
                      >
                        <p className="w-[8.4rem]">Auto-Sync {value ? "Enabled" : "Disabled"}</p>
                      </Switch>
                    </FormControl>
                  );
                }}
              />
            </Tab.Panel>
            <Tab.Panel>
              <SecretSyncDetailsFields />
            </Tab.Panel>
            <Tab.Panel>
              <SecretSyncReviewFields />
            </Tab.Panel>
          </Tab.Panels>
        </Tab.Group>
      </FormProvider>

      <div className="flex w-full flex-row-reverse justify-between gap-4 pt-4">
        <Button onClick={handleNext} colorSchema="secondary" isDisabled={isCreateButtonDisabled}>
          {isFinalStep ? "Create Sync" : "Next"}
        </Button>
        {selectedTabIndex > 0 && (
          <Button onClick={handlePrev} colorSchema="secondary">
            Back
          </Button>
        )}
      </div>
    </form>
  );
};
Repository: Infisical/infisical
File: frontend/src/pages/secret-manager/SecretDashboardPage/SecretDashboardPage.tsx (TypeScript)

Code before:
      // Update URL to match filter state
      navigate({
        search: (prev) => ({
          ...prev,
          search: searchFilter || ""
        })
      });
    },
    [navigate]
  );

  const handleToggleVisibility = useCallback(() => setIsVisible((state) => !state), []);

  // snapshot functions
  const handleSelectSnapshot = useCallback((snapId: string) => {
    setSnapshotId(snapId);
  }, []);

  const handleResetSnapshot = useCallback(() => {
    setSnapshotId(null);
    handlePopUpClose("snapshots");
  }, []);

  const { handleMouseDown, isResizing, colWidth } = useResizableColWidth({
    initialWidth: 320,
    minWidth: 100,
    maxWidth: tableRef.current
      ? tableRef.current.clientWidth - 148 // ensure value column can't collapse completely
      : 800,
    ref: tableRef
  });

  useEffect(() => {
    // restore filters for path if set
    const restore = filterHistory.get(secretPath);
    setFilter(restore ?? defaultFilterState);
    setDebouncedSearchFilter(restore?.searchFilter ?? "");
  }, [secretPath]);

  useEffect(() => {
    if (!routerQueryParams.search && !routerQueryParams.tags && !routerQueryParams.filterBy) return;

    const queryTags = routerQueryParams.tags
      ? (routerQueryParams.tags as string).split(",").filter((tag) => Boolean(tag.trim()))
      : [];
    const updatedTags: Record<string, boolean> = {};
    queryTags.forEach((tag) => {
      updatedTags[tag] = true;
    });

    const filterBy = routerQueryParams.filterBy

Review comment (security):
Filter handlers (handleSearchChange, handleTagToggle, handleToggleRowType) now update both state and URL, creating a bidirectional sync for proper browser history.

Without this change, the browser back button wouldn't work after clicking a secret reference, making the feature a lot less useful.

Code after:

        navigate({
          search: (prev) => ({
            ...prev,
            filterBy: filterByList.length > 0 ? filterByList.join(",") : ""
          })
        });

        return {
          ...state,
          include: newInclude
        };
      });
    },
    [navigate]
  );

  const handleClearFilters = useCallback(() => {
    setFilter({
      searchFilter: "",
      tags: {},
      include: {
        [RowType.Folder]: false,
        [RowType.Import]: false,
        [RowType.DynamicSecret]: false,
        [RowType.Secret]: false,
        [RowType.SecretRotation]: false
      }
    });
    setDebouncedSearchFilter("");
    navigate({
      search: (prev) => ({
        ...prev,
        search: "",
        tags: "",
        filterBy: ""
      })
    });
  }, [navigate]);

  const handleSearchChange = useCallback(
    (searchFilter: string) => {
      setFilter((state) => ({ ...state, searchFilter }));

      // Update URL to match filter state
      navigate({
        search: (prev) => ({
          ...prev,
          search: searchFilter || ""
        })
      });
Repository: Infisical/infisical
File: frontend/src/components/v2/InfisicalSecretInput/InfisicalSecretInput.tsx (TypeScript)

Code before:
        }
      }
    };

    const handlePopUpOpen = () => {
      setHighlightedIndex(-1);
    };

    // to handle multiple ref for single component
    const handleRef = useCallback((el: HTMLTextAreaElement) => {
      // @ts-expect-error this is for multiple ref single component
      inputRef.current = el;
      if (ref) {
        if (typeof ref === "function") {
          ref(el);
        } else {
          // eslint-disable-next-line
          ref.current = el;
        }
      }
    }, []);

    const handleClickSegment = useCallback(
      (segment: string, allSegments: string[]) => {
        if (!projectId) {
          createNotification({
            text: "Project ID is not set",
            type: "error"
          });
          return;
        }

        if (allSegments.length === 0) {
          createNotification({
            text: "Invalid secret reference",
            type: "error"
          });
          return;
        }

        if (allSegments.length === 1) {
          navigate({
            search: (prev) => ({
              ...prev,
              search: segment,
              filterBy: "secret"
            })
          });
          return;
        }


Review comment (security):
two things we should account for in this function:

- permissions - can the user actually view/access this folder/env/secret (can use `permission.can` to check)

- is this actually a secret reference (users can save non-valid "references" that this tries to navigate to even though they don't exists)

Code after:
        }
        if (["ArrowDown", "ArrowUp", "Tab"].includes(e.key)) {
          e.preventDefault();
        }
      }
    };

    const handlePopUpOpen = () => {
      setHighlightedIndex(-1);
    };

    // to handle multiple ref for single component
    const handleRef = useCallback((el: HTMLTextAreaElement) => {
      // @ts-expect-error this is for multiple ref single component
      inputRef.current = el;
      if (ref) {
        if (typeof ref === "function") {
          ref(el);
        } else {
          // eslint-disable-next-line
          ref.current = el;
        }
      }
    }, []);

    const handleClickSegment = useCallback(
      (segment: string, allSegments: string[]) => {
        if (!projectId) {
          createNotification({
            text: "Project ID is not set",
            type: "error"
          });
          return;
        }

        if (allSegments.length === 0) {
          createNotification({
            text: "Invalid secret reference",
            type: "error"
          });
          return;
        }

        if (allSegments.length === 1) {
          const canReadSecretValue = hasSecretReadValueOrDescribePermission(
            permission,
            ProjectPermissionSecretActions.ReadValue,
            {
              environment: propEnvironment ?? "*",
              secretPath: propSecretPath ?? "/",
              secretName: segment,
Repository: Infisical/infisical
File: backend/src/ee/services/audit-log/audit-log-types.ts (TypeScript)

Code before:
  metadata: Record<string, string>; // no metadata yet
}

interface OrgAdminAccessProjectEvent {
  type: EventType.ORG_ADMIN_ACCESS_PROJECT;
  metadata: {
    userId: string;
    username: string;
    email: string;
    projectId: string;
  }; // no metadata yet
}

interface OrgAdminBypassSSOEvent {
  type: EventType.ORG_ADMIN_BYPASS_SSO;
  metadata: Record<string, string>; // no metadata yet
}

interface UserLoginEvent {
  type: EventType.USER_LOGIN;
  metadata: {
    email: string;
    userAgent: string;
    ipAddress: string;
    authMethod: string;
    organizationId?: string;
    organizationName?: string;
    authProvider?: string;
  };
}

interface SelectOrganizationEvent {
  type: EventType.SELECT_ORGANIZATION;
  metadata: {
    email: string;
    userAgent: string;
    ipAddress: string;
    organizationId: string;
    organizationName: string;
    authMethod: string;
  };
}

interface CreateCertificateTemplateEstConfig {
  type: EventType.CREATE_CERTIFICATE_TEMPLATE_EST_CONFIG;
  metadata: {
    certificateTemplateId: string;
    isEnabled: boolean;
  };
}


Review comment (suggestion):
Let's remove `email`, `userAgent`, and `ipAddress` as it's handled by other audit log metadata.

Code after:
  type: EventType.LOAD_PROJECT_KMS_BACKUP;
  metadata: Record<string, string>; // no metadata yet
}

interface OrgAdminAccessProjectEvent {
  type: EventType.ORG_ADMIN_ACCESS_PROJECT;
  metadata: {
    userId: string;
    username: string;
    email: string;
    projectId: string;
  }; // no metadata yet
}

interface OrgAdminBypassSSOEvent {
  type: EventType.ORG_ADMIN_BYPASS_SSO;
  metadata: Record<string, string>; // no metadata yet
}

interface UserLoginEvent {
  type: EventType.USER_LOGIN;
  metadata: {
    organizationId?: string;
    authProvider?: string;
  };
}

interface SelectOrganizationEvent {
  type: EventType.SELECT_ORGANIZATION;
  metadata: {
    organizationId: string;
    organizationName: string;
  };
}

interface CreateCertificateTemplateEstConfig {
  type: EventType.CREATE_CERTIFICATE_TEMPLATE_EST_CONFIG;
  metadata: {
    certificateTemplateId: string;
    isEnabled: boolean;
  };
}

interface GetAzureAdCsTemplatesEvent {
  type: EventType.GET_AZURE_AD_TEMPLATES;
  metadata: {
    caId: string;
    amount: number;
  };
}
Repository: Infisical/infisical
File: backend/src/ee/services/audit-log/audit-log-types.ts (TypeScript)

Code before:
    email: string;
    projectId: string;
  }; // no metadata yet
}

interface OrgAdminBypassSSOEvent {
  type: EventType.ORG_ADMIN_BYPASS_SSO;
  metadata: Record<string, string>; // no metadata yet
}

interface UserLoginEvent {
  type: EventType.USER_LOGIN;
  metadata: {
    email: string;
    userAgent: string;
    ipAddress: string;
    authMethod: string;
    organizationId?: string;
    organizationName?: string;
    authProvider?: string;
  };
}

interface SelectOrganizationEvent {
  type: EventType.SELECT_ORGANIZATION;
  metadata: {
    email: string;
    userAgent: string;
    ipAddress: string;
    organizationId: string;
    organizationName: string;
    authMethod: string;
  };
}

interface CreateCertificateTemplateEstConfig {
  type: EventType.CREATE_CERTIFICATE_TEMPLATE_EST_CONFIG;
  metadata: {
    certificateTemplateId: string;
    isEnabled: boolean;
  };
}

interface GetAzureAdCsTemplatesEvent {
  type: EventType.GET_AZURE_AD_TEMPLATES;
  metadata: {
    caId: string;
    amount: number;
  };
}


Review comment (suggestion):
Let's remove `email`, `userAgent`, and `ipAddress` as it's handled by other audit log metadata.

Code after:
    username: string;
    email: string;
    projectId: string;
  }; // no metadata yet
}

interface OrgAdminBypassSSOEvent {
  type: EventType.ORG_ADMIN_BYPASS_SSO;
  metadata: Record<string, string>; // no metadata yet
}

interface UserLoginEvent {
  type: EventType.USER_LOGIN;
  metadata: {
    organizationId?: string;
    authProvider?: string;
  };
}

interface SelectOrganizationEvent {
  type: EventType.SELECT_ORGANIZATION;
  metadata: {
    organizationId: string;
    organizationName: string;
  };
}

interface CreateCertificateTemplateEstConfig {
  type: EventType.CREATE_CERTIFICATE_TEMPLATE_EST_CONFIG;
  metadata: {
    certificateTemplateId: string;
    isEnabled: boolean;
  };
}

interface GetAzureAdCsTemplatesEvent {
  type: EventType.GET_AZURE_AD_TEMPLATES;
  metadata: {
    caId: string;
    amount: number;
  };
}

interface UpdateCertificateTemplateEstConfig {
  type: EventType.UPDATE_CERTIFICATE_TEMPLATE_EST_CONFIG;
  metadata: {
    certificateTemplateId: string;
    isEnabled: boolean;
  };
}