
Hatch-IT Co., Ltd.The AI’s null fix allows even members: Finding a regression in a small diff
- JavaScript
- AI
- Code review
- Tests
“We’ve made it so it doesn’t crash even with null.” The fix returned by the AI was only a single line. Since passing null actually stopped producing an error, you would be inclined to merge it as is.
However, the proposed fix in this example allows even regular users instead of removing the error.Even if it succeeds on the input you wanted to fix, that does not necessarily mean the rejection conditions that should be upheld have been preserved.This is an example where a smaller diff does not necessarily mean greater confidence.
Using a simplified authorization function, compare the behavior being fixed and the behavior that must be maintained in the same input table. The goal is not to stop after reading the AI’s explanation, but to restate the conditional expression in your own words and demonstrate, with concrete inputs, that a proposed fix cannot be merged.
This is intended for readers who can read JavaScript functions and Git diffs. On November 11, 2026, we reran the published code on macOS and Node.js v26.8.1 and confirmed the output below. The example is a local function that uses fictional data; it is not a production authentication and authorization system.
Include “what must not change” in the success criteria for a fix
The original specification permits only users whose role is admin. The problem is that when user is null, user.role cannot be read, causing an exception.
If the fix request here is limited to just “remove the exception,” half of the specification will be missing. What we need this time is the following state.
| Input | Expected result | Purpose |
|---|---|---|
| {role:'admin'} | true | Continue allowing users who were previously allowed |
| {role:'member'} | false | Continue rejecting users who were previously rejected |
| {} | false | Do not allow access when there is no permission information |
| null / undefined | false | Treat the exception targeted for this fix as a denial |
Even if only the null row passes, that does not mean the entire table has been satisfied. In change reviews, place the behavior that must not be broken next to the symptom you want to fix.
Translate the conditional expression back into Japanese, and the change in meaning becomes visible
The difference between the original code and the proposed example is the following line.
The original expression is “whether role matches admin.” The proposed example changes to “whether user is a truthy value.” In JavaScript, even an empty object is truthy, so both member and objects with no permission information become true.
The exception disappeared because it no longer accesses a property on null. However, at the same time, it has stopped checking role itself. Simply saying “it handles null” does not reveal this second change.
In the corrected version, user?.role === 'admin', if user is null or undefined, it stops retrieving role, compares the resulting undefined with admin, and evaluates to false. Under the current specification, it can handle missing values while retaining the authorization condition.
Reproduce the original exception and compare the proposal and the corrected version using the same input
Save the following as review.mjs in a new practice folder, and run node review.mjs. No additional packages or external accounts are required.
These are the actual results.
If any of the assertions differs from what is expected, an AssertionError is displayed and execution stops with an exit code other than 0. If the output reaches the final PASS line, all checks have passed.
Pay attention to the member and empty lines. The proposed example is true, while the fixed version and expected value are false. You can observe not only that the null exception was reproduced, but also that "the fix broke the previous denial."
The final PASS does not mean that it is safe to adopt the dangerous proposed example. It is the result of the exercise's checks: the original defect was reproduced, a regression in the proposed example was detected, and five cases in the fixed version were verified.
In my diff, target the changed condition with inputs
Even if the change being reviewed is large, you can first narrow it down to a single condition. Write down, one sentence at a time, what it checked before the change and what it checks after the change, and look for an input for which the two answers differ.
In this case, "role is admin" and "the object exists" differ, so member and an empty object are candidates. For an upper-limit check, the candidates would be an input exactly at the limit and one that exceeds it. For handling missing input, candidates might include an empty string and 0. The choice of input can be derived from the meaning of the change.
In Git, use git status --short to check the files in scope, and read unstaged changes with git diff -- and staged changes with git diff --cached --.Untracked new files (lines marked `??` in status) do not appear in a normal diff, so open them directly and check them.
While `git diff --check` is useful for detecting trailing whitespace and conflict markers, it will not find the change to the authorization condition in this case (for this one-line change as well, it reports nothing and exits with code 0). Style checks and tests that read and execute the conditions serve different purposes.
If the result differs from expectations, do not loosen the tests; return to the purpose of the change
| Observed result | Judgment and next action |
|---|---|
| null was fixed, but member is true | Regression in the denial condition. Do not incorporate the fix as-is; return to a change that preserves the allow condition |
| Even admin is false | Check whether the notation of the permission value and the input structure match the assumptions. Do not paper over the issue by widening the permitted range |
| The practice code passes, but it breaks with the real API | Investigate the source, type, and call site of the data passed to the function |
| Unrelated settings and dependencies are mixed into the AI's changes | Confirm the reason null handling is necessary and narrow the changes to those required for the purpose |
This table is the order of investigation when applying the change to the implementation. The real API's connectivity and authentication infrastructure were not verified in this local experiment.
Even with a correct conditional expression, authorization cannot be enforced if the source of the trusted value is different
In this example, `role` is assumed to have been obtained from trusted server-side user information. If `role:'admin'` sent by the browser is trusted as-is, authorization cannot be enforced even if the function's comparison is correct. In an actual service, a decision based on identity verification and server-side permission information is required.
What I want to take away this time is not just the single corrected line, but also a perspective on reviewing.Write one failure you want to fix, along with one allow condition and one denial condition that you want to preserve. Looking at these three on the same input table makes it possible to point out specific semantic changes hidden in the explanation of the fix.
References (primary sources)
Go to list of users who liked
Go to list of comments
Register as a new user and use Qiita more conveniently
- You get articles that match your needs
- You can efficiently read back useful information
- You can use dark theme