Vi granskade Googles zx med och utan en granskningsfärdighet

Färdighetsbänken, del 4 av 4. Samma modell, samma prompt, en färdighet installerad eller inte. Även i serien: en landningssida, en Telegram-bot, och felsökning av ett ormspel.

Tre inlägg in i denna serie har mönstret varit konsekvent: färdighetsarmen gör mer, kontrollerar mer, förbrukar fler tokens och producerar ett mer genomtänkt resultat. Detta inlägg bryter det mönstret. Det är den enda gången i hela bänken där färdighetskörningen blev billigare än baslinjen, och det är också den enda gången färdighetsarmen missade seriens enskilt största fynd. Båda sakerna är sanna samtidigt, och spänningen mellan dem är det mest användbara resultatet vi har fått ut av detta projekt.

Varför Googles zx

Vi ville ha en kodgranskningsuppgift med tre egenskaper: verklig kod, inte ett syntetiskt, buggplanterat kodstycke; något en medelnivåutvecklare rimligen skulle kunna ombes att granska en tisdag; och en kodbas som är tillräckligt välkänd för att läsare ska kunna kontrollera våra påståenden mot den faktiska källkoden istället för att lita blint på oss.

google/zx passade alla tre. Det är Googles bibliotek för att skriva shell-skript i JavaScript, tillräckligt populärt för att en betydande del av Node-verktyg förlitar sig på det, och tillräckligt litet för att en granskning av en enda fil är en rimlig uppgift snarare än ett forskningsprojekt. Vi valde src/core.ts, filen som hanterar själva process-spawning och miljöhantering, hämtad direkt från repositoryt. Inget här är en konstruerad sårbarhet. Det är en verklig, aktivt underhållen fil, och zx är generellt välbyggt: ren struktur, förnuftiga standardvärden på de flesta ställen, den typ av kod som klarar en snabb genomläsning. Det är precis den typ av upplägg där en granskning antingen lönar sig genom att hitta det enda en snabb genomläsning missar, eller så gör den det inte.

Metodik

Två körningar, samma modell, samma instruktion: granska src/core.ts från google/zx och producera fynd rankade P0 till P3, var och en med en plats, vad som går sönder, när det går sönder och en minimal fix. Körning ett fick den instruktionen och inget annat. Körning två fick samma instruktion plus vår egen säkerhetsgransknings- och kodgranskningschecklista, den som levereras i vårt Security Pack och Developer Toolkit, läst i sin helhet innan granskningen påbörjades.

Samma förbehåll som för varje annat inlägg i denna serie: detta är en körning per arm, en modell, en fil. Vi hävdar inte att de specifika antalet fynd replikeras vid en omkörning. Vad vi rapporterar är vad som hände, med verklig telemetri från testbädden, och ett mönster i typen av saker varje arm fångar som vi tror generaliserar bättre än de råa siffrorna gör.

Vad baslinjen hittade

Lämnad ensam läste modellen core.ts och, på eget initiativ, drog även in util.ts och error.ts för kontext innan den skrev något. Ingen bad den om det. Den bestämde att filen inte var meningsfull isolerad och började titta på dess grannar, vilket visade sig vara viktigt.

Baslinjens främsta fynd, och det allvarligaste fyndet i hela bänken, är detta. På rad 139 sätter core.ts sin miljöoption till env: process.env som standard. Det är inte en kopia av miljön, det är en levande referens till den. När du väl vet det är konsekvensen enkel: om någon kodväg gör något som $.env.FOO = 'x', sätter den inte en variabel som är begränsad till det enskilda shell-anropet. Den muterar process.env för hela den körande applikationen, vilket innebär att varje annan del av programmet, varje annat bibliotek, allt som läser miljövariabler efter den punkten, ser ändringen. En konfigurationsjustering avsedd för ett subprocess-anrop läcker in i globalt tillstånd. I en långvarig serverprocess, eller i något skript som sprider ut flera zx-anrop med något annorlunda miljöer, är det den typ av bugg som inte dyker upp i ett snabbt test och sedan korrumperar en helt orelaterad del av systemet dagar senare. Vi verifierade detta direkt mot källkoden snarare än att lita på modellens ord, och raden gör exakt vad fyndet säger.

Resten av baslinjens lista, rankad P2 och P3, var solid utan att vara spektakulär: ett bash-detekteringsfel som tyst sväljs och senare dyker upp som ett missvisande, orelaterat felmeddelande istället för den verkliga orsaken, och ett fall där break() kastar synkront inifrån en .catch()-hanterare, vilket är den typ av kontrollflödesfel som är lätt att skriva och irriterande att felsöka eftersom stackspårningen pekar någonstans oanvändbart.

Fullständig baslinjeräkning: en P1, två P2:or, fyra P3:or.

Vad checklista-armen hittade

Den färdighetsstyrda körningen följde vår kodgranskningschecklista, samma som behandlas i vår sammanställning av säkerhetsfärdigheter, och det lästes som en helt annan granskare: samma fil, samma åtkomst, en helt annan uppsättning problem uppdagades.

Dess bästa fynd var en ohanterad promise-avvisning: anropställen för break() och timeout() anropar kill() utan att avvakta det, så en avvisning från det anropet har ingenstans att landa och kan krascha processen utanför någon try/catch en anropare skrev. Den fångade också en TypeError som uppstår när något försöker async-iterera en ProcessPromise efter att den redan har stoppats, ett verkligt gränsfall som endast visar sig under en specifik sekvens av anrop. Och den flaggade att ZX_PREFIX och ZX_POSTFIX infogas direkt i varje shell-anrop utan sanering, värt en underhållares uppmärksamhet om något av värdena någonsin kan komma från utanför den betrodda skriptförfattaren.

Dessa är verkliga, välformulerade fynd, inte utfyllnad. Checklista-armens rapport var också den mer användbara av de två dokumenten på sina egna villkor: enhetlig struktur, konsekvent allvarlighetsgradsspråk, varje post följde samma form för plats-påverkan-fix utan att modellen behövde uppfinna det formatet på stående fot.

Fullständig checklista-räkning: två P2:or, fem P3:or. Inga P1:or, och anmärkningsvärt nog, inget process.env-fynd alls.

Överlappningen, och hur liten den var

Ställ upp båda listorna och överlappningen är nära noll. Sex baslinjefynd, sju checklista-fynd, totalt tretton, och inte ett enda av dem nämner samma grundorsak. process.env-mutationen. Bash-detekteringssväljningen. Det synkrona kastet i en catch. Den oavvaktade kill(). Den stoppade iteratorns TypeError. Den orenade prefix/postfix-infogningen. Var och en är en annan kodväg.

Det är en större överraskning än någon av de enskilda listorna. Två kompetenta granskare som tittar på samma cirka 300 rader TypeScript, en guidad av en strukturerad checklista och en inte, och de landar på nästan helt åtskilda uppsättningar av problem. Om du hade sagt till oss från början att checklista-körningen i princip skulle dubbelkolla baslinjens lista och lägga till puts, hade vi trott det. Det var inte vad som hände. Checklistan förfinade inte samma sökning, den körde en annan sökning.

Siffrorna

	Baslinje (ingen färdighet)	Färdighetsarm (kodgranskningschecklista)	Delta
Använda tokens	87,899	81,743	−7%
P1-fynd	1	0	−1
P2-fynd	2	2	0
P3-fynd	4	5	+1
Totalt antal fynd	7	7	0
Läste proaktivt närliggande filer	Ja (util.ts, error.ts)	Nej	—
Allvarligaste fynd	process.env-mutation (verifierad)	—	—

Varje annat inlägg i denna serie visade att färdighetsarmen kostade fler tokens för ett mer disciplinerat resultat. Detta är det enda undantaget: checklista-armen var 7% billigare. Vår tolkning är att en checklista begränsar sökutrymmet, och ett smalare sökutrymme är billigare att exekvera, även när det också stänger av några av de vägar en fritt sökande genomgång skulle ha vandrat nerför.

Golv vs. tak

Detta är det skarpaste vi har lärt oss över alla fyra inlägg, och det visar sig tydligast här. Checklistan gjorde granskningen billigare och gav den ett mer rigoröst, mer konsekvent format. Vad den inte gjorde var att hitta process.env-buggen, eftersom det fyndet inte kom från någon kategori på checklistan. Det kom från att baslinjen märkte att core.ts ensam var svår att resonera kring, bestämde sig på egen hand för att hämta util.ts och error.ts, och följde den magkänslan någonstans där checklistans struktur aldrig pekade.

En checklista höjer golvet. Den garanterar en minimistandard för täckning: varje kategori kontrolleras, varje fynd skrivs upp på samma sätt, och du förlorar inte en enkel fångst på grund av en dålig dag. Den höjer inte taket. Det bästa möjliga fyndet på en given fil kan ligga utanför varje kategori som checklistan räknar upp, och en process som bara följer checklistan kommer att gå rakt förbi det, självsäkert, i en snyggt formaterad rapport.

Det är inte ett argument mot checklistor. Noll av checklistans sju fynd var dåliga, och två av dem var den typ av saker en upptagen mänsklig granskare rimligen missar under tidspress. Det är ett argument för att veta vad en checklista är till för. Det är ett golvhöjande verktyg, inte ett takhöjande, och att behandla det som båda är hur en verklig P1 slinker igenom en granskning som annars ser grundlig ut.

Hur du själv kör en tvåstegsgranskning

Med tanke på vad vi hittade är vår faktiska rekommendation inte "använd en checklista" eller "hoppa över checklistan". Det är att köra båda, på allt som är viktigt.

Börja med en fri genomgång. Peka modellen mot filen, ge den granskningsinstruktionen och låt den läsa vad den vill läsa. Ge den ingen bedömningsmall. Detta är den genomgång som har bäst chanser att fånga det som ingen tänkte lägga på en lista, eftersom den inte är begränsad till listan.

Kör sedan en andra, separat genomgång med en strukturerad checklista, vår eller din egen. Detta är den genomgång som garanterar täckning: kategorierna som är tråkiga att kontrollera för hand men lätta att hoppa över när du följer en magkänsla, sanering, felhantering, resursstädning, gås igenom varje gång.

Jämför de två rapporterna innan du läser någon av dem som slutgiltig. Om vårt överlappningsnummer håller på din kod på samma sätt som det höll på zx:s, förvänta dig att de två listorna delar mindre än hälften av sina fynd. Behandla det som det förväntade resultatet, inte ett tecken på att någon genomgång misslyckades. Det fullständiga testprotokollet beskriver mer i detalj hur vi strukturerar denna typ av parvis körning, inklusive hur vi kontrollerar att modellen läser sin egen tidigare utdata.

Om du bara har budget för en genomgång, är vårt ärliga råd baserat på detta resultat: kör den fria genomgången först. Det är den som är mer benägen att hitta det fynd du inte visste att du skulle leta efter. Sedan, om tiden tillåter, följ upp med checklistan för att säkerställa att inget tråkigt missades. För en bredare bild av vilka färdigheter som hanterar denna typ av granskning väl, se vår sammanställning av bästa kodningsfärdigheter.

FAQ

Är process.env-fyndet en verklig sårbarhet i zx? Det är ett verkligt beteende värt en underhållares uppmärksamhet, inte en offentliggjord CVE och inte något vi framställer som en aktiv exploit. zx är ett välbyggt, aktivt underhållet bibliotek överlag, och detta är ett designval, en levande referens istället för en kopia, som har en verklig mutationskonsekvens för varje kodväg som skriver till $.env. Vi verifierade raden själva mot källkoden snarare än att lita på modellens rapport, vilket är exakt varför vi är bekväma med att kalla det det främsta fyndet i hela bänken.

Varför kostade färdighetsarmen mindre här, när den kostade mer överallt annars i denna serie? Vår bästa förklaring är omfattning. En designfärdighet i landningssidan inbjuder till iteration: kontrollera utdata, revidera, kontrollera igen. En granskningschecklista fungerar annorlunda. Den definierar en fast uppsättning kategorier att gå igenom en gång, vilket begränsar sökningen snarare än att utöka den. Smalare sökning, färre tokens. Det är en plausibel mekanism, inte en bevisad, eftersom detta är en enskild körning.

Ska jag lita på en checklista-guidad AI-kodgranskning för att fånga allt? Nej, och det är det viktigaste fyndet här. En checklista är en golvhöjande anordning: den garanterar en konsekvent minimigenomgång över kända kategorier. Den är inte en takhöjande anordning, och det största fyndet i hela denna bänk kom från en genomgång som inte följde en. Använd en checklista för täckning och konsekvens. Använd den inte som din enda genomgång på kod som verkligen är viktig.

Vad betyder detta för att välja en Claude-kodgranskningsfärdighet i praktiken? Behandla inte "vilken färdighet" som det enda beslutet. Behandla "hur många genomgångar" som det viktigare beslutet. En bra checklista-färdighet, som den i vår kodgranskningslista, är värd att installera för den konsekvens och de kategorier den garanterar. Kombinera den med minst en oledd genomgång på allt du faktiskt skulle leverera, och läs vår täckning av säkerhetsfärdigheter för hur vi väger granskningsfärdigheter mot varandra i katalogen.

Det var serien. Fyra uppgifter, fyra ärliga utlåtanden, och den röda tråden genom alla är densamma: en färdighet ändrar vad en modell kontrollerar, inte om den är kapabel till arbetet. Om den avvägningen är värd det beror helt på vad du bygger och hur noga någon kommer att titta på det efteråt.