Open-source contribution / Engineering story

GROWI

A null check, a client contract, and who gets to know a page exists.

Two merged fixes to the page API of GROWI, an open-source wiki platform. Found by reproducing a failure, shaped by maintainer review, and settled by a security property the first version of the fix had not accounted for.

01 Situation

GROWI is an open-source wiki and knowledge-management platform, developed in a large pnpm monorepo by GROWI Labs and outside contributors.

On the main branch, sending the page rename API a syntactically valid ObjectId that belonged to no page produced an HTTP 500 with a generic “unknown” error.

02 Observation

The route read page.descendantCount before checking whether the lookup had returned anything. The lookup returns null both for a page that does not exist and for one the caller is not allowed to see, so an ordinary missing page became an unhandled TypeError, swallowed into a 500.

I filed the issue with a reproduction and that diagnosis before writing a fix. A maintainer confirmed the root cause and traced it to a line inserted above a pre-existing null check.

03 Constraint

This was someone else’s system, with its own conventions, client code, and reviewers. The server route was one layer of it: the client decides which message to show from the error code, and every translation depends on that code.

The same pattern also existed in the page duplicate route.

04 Decision

Land the rename fix first and on its own, as the maintainer preferred, and handle the duplicate route in a separate follow-up so that each review stayed small.

05 Tradeoff

The first version introduced more specific error codes. Review showed their cost: the client’s localized error rendering switched on the existing notfound_or_forbidden code, so new codes would have fallen through to an untranslated fallback in every language. I kept the code and message and changed only the HTTP status.

Later, while reviewing the follow-up, the maintainers revisited the 403/404 split itself. A signed-in user without permission could use the difference to learn whether a page exists. The project settled on a uniform 404 for page operations, and I changed the follow-up to match.

06 Execution

Null handling before the dereference in both routes; OpenAPI responses aligned with the real behaviour; database-backed integration tests for missing, forbidden, and successful requests.

During review, the tests were reworked to exercise the real page lookup instead of mocking it, and implementation-specific assertions and casts were removed.

07 Verification

For the follow-up: focused duplicate-route integration tests passed 4/4, run twice independently; the related page API suite passed 29/29; type checking and OpenAPI validation passed.

08 Result

#11615 merged on 10 August 2026 and closed the issue. #11753 merged on 25 August 2026, returning a uniform 404 for unavailable source pages. The maintainers applied the same policy to other page write routes in a separate change.

09 Reflection

A fix can be correct at the layer you are looking at and still be wrong for the system. Each round of review moved the question outward: from a null check, to a client contract, to what an API reveals to someone who should not see a page.

What this shows

  • Understand the existing system before changing it.
  • A fix is only correct if the rest of the system agrees.
  • Prefer evidence over confidence.
  • Keep changes small enough to review honestly.

Timeline

  1. Investigation Reported: rename API returns 500 for a missing page Filed GROWI #11572 with a reproduction and a diagnosis: the route dereferenced the page before checking whether the lookup returned null.
  2. Upstream merge Rename fix merged upstream GROWI #11615 merged: missing and inaccessible pages no longer produce a 500 from the rename API. Regression tests exercise the real lookup path.
  3. Review Policy changed mid-review; follow-up merged While reviewing the duplicate-route fix, GROWI’s maintainers decided a 403/404 split lets unauthorized users learn whether a page exists. The fix was changed to a uniform 404 and merged the same day.

Public evidence