Rank tracking: enforce config cap on unarchive, raise cap to 500 (#347)

The archived-config reactivation path in createConfig returned before the
MAX_CONFIGS_PER_PROJECT check, so re-adding previously archived domains
could push a project past the active-config cap (Codex security finding
ba873417). The cap check now runs before both the reactivation and the
new-row insert. Also raises MAX_CONFIGS_PER_PROJECT from 100 to 500.
This commit is contained in:
Ben Senescu 2026-07-05 17:39:53 -04:00 committed by GitHub
parent c993ef4931
commit 7c64cd3c5c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 45 additions and 23 deletions

View File

@ -45,6 +45,7 @@ describe("RankTrackingService.createConfig", () => {
it("reactivates an archived config instead of throwing, applying the new settings", async () => { it("reactivates an archived config instead of throwing, applying the new settings", async () => {
mocks.getConfigByProjectDomainLocation.mockResolvedValue(archivedConfig); mocks.getConfigByProjectDomainLocation.mockResolvedValue(archivedConfig);
mocks.getConfigsForProject.mockResolvedValue([]);
mocks.updateConfig.mockResolvedValue(undefined); mocks.updateConfig.mockResolvedValue(undefined);
const { RankTrackingService } = await import("./RankTrackingService"); const { RankTrackingService } = await import("./RankTrackingService");
@ -83,6 +84,25 @@ describe("RankTrackingService.createConfig", () => {
expect(mocks.createConfig).not.toHaveBeenCalled(); expect(mocks.createConfig).not.toHaveBeenCalled();
}); });
it("rejects reactivating an archived config when the project is at the active-config cap", async () => {
const { MAX_CONFIGS_PER_PROJECT } = await import("@/shared/rank-tracking");
mocks.getConfigByProjectDomainLocation.mockResolvedValue(archivedConfig);
mocks.getConfigsForProject.mockResolvedValue(
Array.from({ length: MAX_CONFIGS_PER_PROJECT }, (_, i) => ({
...archivedConfig,
id: `config_${i}`,
isActive: true,
})),
);
const { RankTrackingService } = await import("./RankTrackingService");
await expect(
RankTrackingService.createConfig(baseInput),
).rejects.toMatchObject({ code: "VALIDATION_ERROR" });
expect(mocks.updateConfig).not.toHaveBeenCalled();
expect(mocks.createConfig).not.toHaveBeenCalled();
});
it("creates a new config when none exists for the domain + location", async () => { it("creates a new config when none exists for the domain + location", async () => {
mocks.getConfigByProjectDomainLocation.mockResolvedValue(null); mocks.getConfigByProjectDomainLocation.mockResolvedValue(null);
mocks.getConfigsForProject.mockResolvedValue([]); mocks.getConfigsForProject.mockResolvedValue([]);

View File

@ -50,19 +50,31 @@ async function createConfig(input: {
normalizedDomain, normalizedDomain,
locationCode, locationCode,
); );
if (existing) { // The (project, domain, location) row still exists when a domain is
// The (project, domain, location) row still exists when a domain is // archived — archiving only flips isActive to false. So re-adding an
// archived — archiving only flips isActive to false. So re-adding an // archived domain reactivates that row (keeping its keyword/ranking
// archived domain reactivates that row (keeping its keyword/ranking // history) with the freshly chosen settings, rather than colliding with
// history) with the freshly chosen settings, rather than colliding with // the unique index. An already-active row is a genuine duplicate.
// the unique index. An already-active row is a genuine duplicate. if (existing?.isActive) {
if (existing.isActive) { throw new AppError(
throw new AppError( "VALIDATION_ERROR",
"VALIDATION_ERROR", "This domain + country combination is already being tracked",
"This domain + country combination is already being tracked", );
); }
}
// Enforced for reactivations too, not just new rows — otherwise archiving
// and re-adding domains would push a project past the active-config cap.
const allConfigs = await RankTrackingRepository.getConfigsForProject(
input.projectId,
);
if (allConfigs.length >= MAX_CONFIGS_PER_PROJECT) {
throw new AppError(
"VALIDATION_ERROR",
`Maximum ${MAX_CONFIGS_PER_PROJECT} tracked domains per project`,
);
}
if (existing) {
await RankTrackingRepository.updateConfig(existing.id, input.projectId, { await RankTrackingRepository.updateConfig(existing.id, input.projectId, {
isActive: true, isActive: true,
languageCode: input.languageCode ?? "en", languageCode: input.languageCode ?? "en",
@ -78,16 +90,6 @@ async function createConfig(input: {
return { configId: existing.id }; return { configId: existing.id };
} }
const allConfigs = await RankTrackingRepository.getConfigsForProject(
input.projectId,
);
if (allConfigs.length >= MAX_CONFIGS_PER_PROJECT) {
throw new AppError(
"VALIDATION_ERROR",
`Maximum ${MAX_CONFIGS_PER_PROJECT} tracked domains per project`,
);
}
const configId = crypto.randomUUID(); const configId = crypto.randomUUID();
await RankTrackingRepository.createConfig({ await RankTrackingRepository.createConfig({

View File

@ -37,7 +37,7 @@ export const SECONDS_PER_BATCH = 6;
export const MAX_KEYWORDS_PER_CONFIG = 1000; export const MAX_KEYWORDS_PER_CONFIG = 1000;
/** Maximum configs (domain+location combos) per project */ /** Maximum configs (domain+location combos) per project */
export const MAX_CONFIGS_PER_PROJECT = 100; export const MAX_CONFIGS_PER_PROJECT = 500;
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
// Cost estimation // Cost estimation