Secure code review with an AI coding agent¶

If you ask an AI assistant to help review an unfamiliar codebase for security problems, the hard part is not the reasoning. It is getting the right few hundred lines in front of the model out of several hundred thousand. A reviewer who already knows the codebase greps for check_permission. A reviewer who has never opened it does not know that string exists.

This page is two real review sessions against two real codebases — one where CodeNavi has no call graph and one where it does — with the actual queries and the actual results, including the query that missed and what the tool said about it. Nothing here is a reconstruction.

The first codebase: Bugzilla¶

Bugzilla, Mozilla's bug tracker, at revision 4299886. As CodeNavi indexes it:


Files indexed	760
Chunks	5011
Perl	3836 chunks across ~290 files
Template Toolkit	565 chunks
Everything else	JavaScript, CSS, reStructuredText, a little Python

It is a useful subject for two reasons. It is large enough that no reviewer reads it end to end, and it is mostly Perl and Template Toolkit — file types most code-intelligence tooling does not parse at all. If your estate contains a system like this, the tool that cannot read it is not a slower option; it is not an option.

Question 1 — where is the authorization gate?¶

The first thing a reviewer wants on a multi-tenant-ish system is the function that decides who may see a record. Phrased as a question rather than a symbol:

decide whether the current user is allowed to see a given bug

Top hit, Bugzilla/Bug.pm:415-433:

sub check_is_visible {
  my ($self, $input_id) = @_;
  $input_id ||= $self->id;
  my $user = Bugzilla->user;

  if (!$user->can_see_bug($self->id)) {
    # The error the user sees depends on whether or not they are
    # logged in ...
    if ($user->id) {
      ThrowUserError("bug_access_denied", {bug_id => $input_id});
    }
    else {
      ThrowUserError("bug_access_query", {bug_id => $input_id});
    }
  }
}

That is the gate, found without knowing the words check_is_visible, can_see_bug, or Bug.pm. It refuses rather than filters, and the message varies by login state: a logged-in user is denied, an anonymous one is told to log in.

It is tempting to read that branch as anti-enumeration. It is not — and the first draft of this page said it was. Forty lines up, Bugzilla::Bug->check (Bug.pm:372-403) resolves the id first and throws bug_id_does_not_exist before the visibility gate runs, so "does not exist" and "not authorized to access bug #N" are different messages for everyone. The existence of a restricted bug is disclosed. Bugzilla ids are sequential and appear in every URL, so this is an accepted design rather than a defect, but it belongs in the report as an observation — not as praise. Reading the caller is what corrects the reflex.

Question 2 — the one that missed¶

Next question, CSRF protection. Phrased the way the OWASP category is phrased:

protect a form submission against cross-site request forgery with a token

This came back weak. The top hit scored 0.005 and was a login handler concerned with cross-domain GET, not CSRF tokens. The response carried a low_signal diagnostic — CodeNavi's own signal that the best hit fell below the floor of queries that have real signal behind them.

The right move at that point is not to fall back to grep. It is to re-ask in the codebase's vocabulary — this system does not say "CSRF", it says "token":

issue a token for a session and verify it matches before performing the action

Score 0.99, Bugzilla/Token.pm:412-423:

# Given a token, makes sure it comes from the currently logged in user
# and match the expected event. Returns 1 on success, else displays a warning.
sub check_token_data {
  my ($token, $expected_action, $alternate_script) = @_;
  ...
  my ($creator_id, $date, $token_action) = GetTokenData($token);
  unless ($creator_id
    && $creator_id == $user->id
    && $token_action eq $expected_action)

Two queries, one of which failed. That is what the work actually looks like, and a tool that tells you its answer is weak is more useful than one that returns five confident irrelevant results.

The follow-up question — is this actually called everywhere it should be? — is answered by enumeration: check_token_data and check_hash_token appear at 75 call sites across 30 files, each naming its own action:

editusers.cgi:422    check_token_data($token, 'delete_user');
editgroups.cgi:267   check_token_data($token, 'edit_group');
editflagtypes.cgi:404 check_token_data($token, 'delete_flagtype');

Per-action naming means a token minted for one action cannot authorise a different one. Note what this evidence is and is not: an enumeration of call sites, not a proof that every state-changing path reaches one.

Question 3 — the answer that is neither pass nor fail¶

build a SQL WHERE clause from user supplied search criteria

Top hit, Bugzilla/Search.pm:756-789, the query builder. The statement is assembled by string interpolation and executed without bind values (Bugzilla/Search.pm:745):

  my $query = <<END;
SELECT $select
  FROM $from
 WHERE $where
$group_by$order_by$limit
END

The reflex is to call that an injection finding. It is not, and stopping at the top hit is how a reviewer files a wrong report. The values reaching $where pass through $dbh->quote() — 15 call sites in that one file — and numeric inputs through detaint_natural. Elsewhere the codebase uses placeholders properly; Bugzilla/User.pm:2435 is DELETE FROM login_failure WHERE user_id = ? AND ip_addr = ?.

So the honest verdict is partial: driver-level escaping rather than parameterisation, in the one place where the shape of the query is itself user-driven, and a static search cannot prove no value reaches that string by another route. That verdict is more useful to the team than either "SQL injection found" or "looks fine".

Question 4 — the same question, on a codebase with a call graph¶

Question 2 ended at an enumeration: 75 call sites is where the token check is called, not proof that every path that needs it reaches one. On Perl that is the ceiling. The second session asks the access-control question on Medusa, a TypeScript commerce platform, at revision 782957b4 — a language where CodeNavi builds a call graph. The index root is packages/; every path below is relative to it.


Indexed source files	4,903 `.ts` · 2,047 `.tsx` · 120 `.js`
Indexed, no call graph	220 JSON, 131 Markdown
Admin API route files	240

authenticate an admin API request and reject it when the user is not logged in

The top hit was a test file (core/framework/src/http/__tests__/index.spec.ts, 0.03). That is what a general answer to a specific question looks like, and it is why the advice is to read the top three, not the top one. The second hit is the gate, core/framework/src/http/middlewares/authenticate-middleware.ts:35-118, ending with the only refusal in the file:

    // If we allow unauthenticated requests (i.e public endpoints), just continue
    if (options.allowUnauthenticated) {
      return next()
    }

    res.status(401).json({ message: "Unauthorized" })

The fourth hit is the more interesting one: core/framework/src/http/router.ts:306-344, #applyAuthMiddleware, which wraps the gate per namespace and skips it when the matched route has optedOutOfAuth (:327). A framework-level mount with a per-route escape hatch changes the question from Bugzilla's — no longer does every route call this? but which routes opt out?

Structural first. find_callers("authenticate") returns 41 call sites. Read honestly: 24 of them are a different authenticate — ApiKeyModuleService's method and the auth-provider test suites share the name, and the tool's own output says its edges are name-based. The one that matters is router.ts:316, inside ApiLoader. Reading its caller, load(), at router.ts:495-499:

    this.#applyAuthMiddleware(routesFinder, "/admin", "user", [
      "bearer",
      "session",
      "api-key",
    ])

The entire /admin namespace is gated once, at load time; a new admin route inherits the gate without calling anything. /store gets the same middleware with allowUnauthenticated: true (:514-522) — the storefront is public by design and customer identity is optional.

Then the escape hatch. The loader turns a route file's export const AUTHENTICATE = false into optedOutOfAuth (core/framework/src/http/routes-loader.ts:149), so the exceptions are a literal search:

export const AUTHENTICATE\s*=\s*false under medusa/src/api/

2 matches in 240 admin route files: admin/feature-flags/route.ts:4 and admin/invites/accept/route.ts:40. Both got read.

feature-flags returns every feature-flag key and value to an unauthenticated caller (:9-26). Probably deliberate — the dashboard needs them before login — and still an information-disclosure observation for the report: which backend features are switched on is public.
invites/accept opts out of the default gate and immediately re-applies a variant of it: authenticate("user", ["session", "bearer"], { allowUnregistered: true }) at medusa/src/api/admin/invites/middlewares.ts:58-60. An anonymous request still gets the 401 at authenticate-middleware.ts:114; what is admitted is an authenticated identity with no user record yet — an invitee. The handler then refuses anyone who already has one (accept/route.ts:14-19), and the workflow validates the invite token, 401 on failure (:29-35). Three checks where the grep showed zero.

That second bullet is the point of the call graph. find_callers had already listed invites/middlewares.ts:58 as a caller of the gate; the grep for opt-outs and the caller list intersect on one file, and the intersection is the compensating control. On Bugzilla, that reasoning was not available.

Last, blast radius — the question a reviewer asks before recommending a change to the gate. impact_analysis("authenticate", depth=2): 21 files, 7 of them tests, three hubs flagged for first attention (ApiLoader, createServer, the auth GET handler). And the tool's own caveat, verbatim:

Graph coverage: this result is a LOWER BOUND — … callers unnamed for 64% of ts call sites.

That number is real and it is conservative: 42% of Medusa's TypeScript call edges are in test files, which are anonymous describe/it callbacks by nature, and the non-test rate is 40%. The product reports the all-files figure because a risk tool that rounds toward "safe" is the wrong tool.

Verdict for the sub-check authentication is on the execution path: pass — framework-mounted, opt-outs enumerable, each opt-out read and accounted for. Verdict for the rule: partial, because the other sub-checks (per-resource policies, object-level checks on :id routes, customer scoping on the store side) were not run, and a scoped run says so.

The output: an evidence-linked gap report¶

The point of a session is not the transcript, it is an artifact. CodeNavi ships rule packs (OWASP Top 10, OWASP ASVS core, EU Cyber Resilience Act Annex I) that drive the same loop per rule and produce a table with a verdict and file:line evidence for each. The three Bugzilla rows and the one Medusa row from the sessions above:

Rule	Verdict	Evidence
TOP10-A01 Broken Access Control	partial	`Bugzilla/Bug.pm:415-433`, `Bugzilla/Token.pm:414-423`, 75 call sites
TOP10-A05 Injection	partial	`Bugzilla/Search.pm:781-786`, `:745`, `:2027,2138,2554`
TOP10-A07 Authentication Failures	partial	`Bugzilla/User.pm:2416-2448`, `Bugzilla/Auth.pm:197-205`
TOP10-A01 Broken Access Control — Medusa	partial	`router.ts:495-499`, `routes-loader.ts:149`, 2 opt-outs in 240 route files, `invites/middlewares.ts:58-60`

Each report ends with a self-hash footer so the artifact carries a verifiable fingerprint — head -n -1 report.md | sha256sum must equal the hex in the last line. That is tamper and corruption detection, not a signature.

What this does not do¶

Being specific about the limits is what makes the rest worth reading.

Static only. No request touches a running instance. Every finding is a candidate for a human to confirm.
Enumeration is not reachability. "75 call sites" answers where is this called, not does every path that needs it call it. On languages where CodeNavi builds a call graph, find_callers and impact_analysis narrow that gap; Perl is not one of them (see supported languages), so on Bugzilla the enumeration is the ceiling. Question 4 is the same question asked where the graph exists.
Call edges are name-based. find_callers returned 41 sites for authenticate; 24 were a different function with the same name. The tool says so in its output, and the reviewer still has to read.
A scoped run is a scoped run. The Bugzilla report covers 3 of 10 rules and the Medusa report 1 of 10; both say so on their title line. A rule with no row is unchecked, not passed.
Semantic search returns near matches. False positives are expected; that is why every row cites lines a human can open.

Reproducing this¶

Point CodeNavi at either checkout, wait for the index, and ask the questions in the order above. The queries are in this page verbatim. The full report, including the rules this excerpt omits, is produced by the compliance-check skill shipped with the Claude Code plugin.