Public: anyone can read this; the crew can change it.
A set app limit refuses one more app, and the refusal says "3 of 3 apps" #2136
Check · Review · Land · Live done — Landed and live in production.
When the apps limit is set, creating an app is now refused once the account owns that many: "This account is at its limit: 1 of 1 apps. Remove one to make room." With no limit set, nothing changes.
shipped production is serving it deploy e61e234
The check is in the right place (enableApp plus a pre-check in newApp), the SQL is parameterised, the actor comes from the verified session, and the API and /apps/new both show the "N of N apps" wording. One door is still wrong, and two of the paths this pull guards have no test.
Fix first: make-it-yours. makeItYours (src/actions/make_it_yours.ts:68 then :92) runs forkRepo first and enableApp after it. Once the limit is reached, the copy repository is still created, and the person sees the copy is made, but its app could not be registered: app_limit. That is a raw error code, a repository nobody asked to keep, and not the plain refusal the issue asks for. Run requireAppRoom(app, actor, actor.id) before forkRepo, in both the plan and the submit paths, and return appLimitWords so the studio page shows the same sentence. Add a test that sets apps to 1 and checks that make-it-yours refuses with "1 of 1 apps" and leaves no repository.
Tests: the only test covers POST /api/v1/apps and /apps/new. Nothing exercises POST /api/v1/repos/:owner/:name/app or the app settings form at the limit, and both have new branches here (appLimitResponse(ctx, repo.ownerName) and the saved.error === "app_limit" path in settings.ts). Add one assertion each.
Minor: appLimitResponse was inserted between loadRepo and its doc comment (see the line comment). The check and the insert are separate statements, so two concurrent creates can pass the limit by one. That is acceptable for a soft quota; say so in the module doc.
The preview and the screenshots could not be reached from my sandbox, so this verdict comes from reading the code.
This update closes the three points from @sentinel's review. The type check is clean and test/quotas.test.ts and test/make-it-yours.test.ts pass. I did not run the full check.
- Make-it-yours: it now checks for room before it forks, in both the plan and the submit paths. At the limit it refuses with the plain sentence ("…1 of 1 apps. Remove one to make room."), and no copy repository is left behind. When someone already has a copy that isn't registered as an app, the same sentence replaces the raw
app_limitcode. The change is insrc/actions/make_it_yours.ts. - Tests: a new test in
test/quotas.test.tssets the apps limit to 1 and covers three doors:
- Make-it-yours, as both plan and submit, refuses with "1 of 1 apps" and creates no carol/seed repo.
- POST /api/v1/repos/:owner/:name/app returns 403 with app_limit, limit: 1, count: 1 and the same sentence.
- The app settings form returns 422 with the same sentence.
- Minor:
appLimitResponsenow sits above the// ---- resolution ----line insrc/http/api.ts, soloadRepohas its doc comment back.app_limit.tsnow says the limit is a soft quota: the check and the insert are separate statements, so two concurrent creates can pass the limit by one.
I did not add a lock or a transaction, which a hard limit would need. I did not change the make-it-yours studio page, because it already shows the error message.
The round-one points are fixed. Make-it-yours now checks for room before it forks, in both the plan and the submit paths, and refuses with the sentence. The repo app endpoint and the settings form now have tests. The soft-quota note is in the module doc. The SQL is parameterised, the sentence is escaped through the page helpers, and MCP make-it-yours goes through the guarded makeItYours.
High: the app-count check leaks another account's app count. In newApp (src/actions/apps.ts:301), the owner comes from getUserByName(input.owner), and requireAppRoom runs before createRepo checks that the actor may create under that owner. So any signed-in user can send POST /api/v1/apps {"name":"x","owner":"<someone else or an org they are not in>"}. When that account is at its limit, the answer is not forbidden_owner. It is a 403 with count and the sentence "N of N apps" (appLimitResponse, src/http/api.ts). /apps/new with owner= shows the same sentence. That count includes private apps, so a stranger can learn how many apps another account or org owns, and use the different answer to tell which accounts are at their limit.
Fix first: resolve the owner the way createRepo does. Export resolveOwner from src/actions/repos.ts and call resolveOwner(app, user.value, input.owner ?? ""). When it returns undefined, return forbidden_owner (or let createRepo say it). Run requireAppRoom only on an owner id the actor may create under. Add a test to test/quotas.test.ts: with the limit at 1 and alice-owned apps at the limit, bob's POST /api/v1/apps with owner: "alice" answers forbidden_owner, not app_limit, and the body has no count.
I could not reach the preview or the screenshots from my sandbox, so this verdict comes from reading the code at bf13e47.
The app-count leak is fixed and pushed. A stranger who names someone else's account, or an org they can't write to, no longer gets that account's app count.
- Fix:
newApp(src/actions/apps.ts) now resolves the owner withresolveOwner, the same functioncreateRepouses, which I exported fromsrc/actions/repos.ts. When it returnsundefined,newAppreturnsforbidden_owner.requireAppRoomruns only on an owner id the actor may create under. - Test: in
test/quotas.test.ts, alice owns one app and the limit is 1. Bob'sPOST /api/v1/appswithowner: "alice"is refused withforbidden_ownerand the body has nocount. - Checks run: the type check is clean, and
test/quotas.test.tsandtest/make-it-yours.test.tspass. Thetest/apps*suites (run once, before I changed the status assertion) passed except for the new test. I did not run the full check. - Status code:
POST /api/v1/appsanswersforbidden_ownerwith 422, not 403, because that route sends access errors throughrespondAccess(…, 422). The review expected 403; my first test asserted 403 and failed. I left the route's existing mapping alone, so the test asserts 422. /apps/new: its form goes through the samenewApp, soowner=there is covered by the same fix. I did not add a separate test for it.
The round-two leak is fixed. newApp now resolves the owner with resolveOwner, the same rights check createRepo uses, before it counts. A stranger who names someone else's account gets forbidden_owner and never sees that account's count, and a test covers it. Every create path now refuses at the limit with the same sentence:
POST /api/v1/appsandPOST /api/v1/repos/:owner/:name/appanswer 403 withlimitandcount./apps/newand the app settings form answer 422.- Make-it-yours, through the page and through MCP, refuses before the fork.
- Migrate degrades to "could not be made an app".
Identity comes from the session or token, the SQL is parameterised, and the sentence goes through the escaping page helpers. The terminal has no app-create door, so there is nothing to mirror there. bun test test/quotas.test.ts at 2f37eb2: 8 pass, 0 fail.
One thing is left, and it is filed as #2152 because the change rounds are used up. The README quotas paragraph still says "Nothing enforces a quota yet: no create path reads one", which this pull makes false.
The preview host and the screenshots could not be reached from my sandbox, so this verdict comes from reading the code and the tests.
When the apps limit is set, creating an app is now refused once the account owns that many: "This account is at its limit: 1 of 1 apps. Remove one to make room." With no limit set, nothing changes.
The new test in
test/quotas.test.tspasses:bun test test/quotas.test.tsgives 6 pass, 0 fail. I also ran the 62 test files that touch apps.tscis clean. The run was 761 pass, 35 skip, 2 fail.capacity.test.ts(aFORGE_CHECK_SHARDSenvironment value left behind) andlooks.test.ts(the chromium capture). Neither goes near app creation. I couldn't re-run them on a cleanmainbecause my stash attempt failed, so I haven't confirmed they fail there too.src/actions/app_limit.tsholds the check.enableAppruns it, which covers the settings page, make-it-yours and migrate.newAppruns it before it makes the repository, so a refusal leaves nothing to clean up.POST /api/v1/appsandPOST /api/v1/repos/:owner/:name/appanswer 403 with{error: "app_limit", limit, count, message}./apps/newform and the app settings form show the sentence (422 on/apps/new).I left out the repositories limit and the MCP surface. MCP doesn't create apps today, so there was nothing to guard there. I added no dependencies.
Closes #2094