Lägg till dokumenterad rättelse för auditloggens integritet#101
Merged
Conversation
Ett tidigare fel (felaktigt tolkade ingående balanser, sedan patchade direkt i databasen) visade att vi saknade ett sätt att förklara en känd, redan utredd avvikelse i hashkedjan utan att antingen (a) tysta integritetskontrollen permanent genom att skriva om entry_hash/ previous_hash, vilket gör kedjan om intet, eller (b) låta ett förklarat historiskt fel blockera rapportexport och andra åtgärder för evigt. recordIntegrityRemediation(companyId, auditLogId, reason) löser detta genom att lägga till en ny, normalt kedjad INTEGRITY_REMEDIATION-post som pekar ut och förklarar den trasiga raden - originalraden rörs aldrig. validateIntegrity() delas nu upp i två nivåer: en dokumenterad avvikelse räknas inte längre som kritisk (och blockerar därmed inte ReportIntegrityService), men förblir synlig via den nya listDocumentedExceptions(companyId). Rättelsen kräver att målraden faktiskt har ett odokumenterat problem just nu, så metoden inte kan användas för att stämpla friska rader i förväg. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Skriver ner regeln som den nya rättelsemekanismen (och det bakomliggande incidenten den löser) bygger på, så framtida agent-sessioner inte återupprepar misstaget: aldrig patcha ledgertabeller direkt med SQL, rättelser går via applikationens egna domänoperationer (samma append-only-mönster som korrigeringsverifikationer), och en upptäckt hashintegritetsavvikelse dokumenteras via AuditLogService.recordIntegrityRemediation() - inte genom att skriva om entry_hash/previous_hash eller köra rebuildIntegrityChain()/ repairIntegrityForAllCompanies() som allmänna reparationsverktyg. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
P1: exemptionen nycklades bara på auditLogId, så när en rad väl fått en rättelsepost flyttades varje nuvarande OCH framtida hash-/föregående- hash-avvikelse på den raden ur validateIntegrity()'s blockerande lista - oavsett om den nya avvikelsen hade något med den ursprungligen förklarade att göra. En andra, annorlunda manipulation av en redan rättad rad maskerades därmed tyst. recordIntegrityRemediation() fångar nu radens exakta avvikande tillstånd vid rättelsetillfället (lagrat entry_hash, lagrat previous_hash, samt den hash som innehållet borde ge om det inte var manipulerat) och lagrar det i rättelsepostens details-text. checkIntegrityForCompany() undantar en rad bara när dess NUVARANDE tillstånd exakt matchar en tidigare dokumenterad avvikelse - ändras raden igen, på något sätt, matchar inget tidigare dokumenterat tillstånd längre och den flaggas som kritisk på nytt. Nytt regressionstest bevisar detta: rättar en rad, manipulerar den sedan en andra gång på ett annat sätt, och verifierar att den dyker upp i den kritiska listan igen i stället för att tystas av den första rättelsen. Justerade också en CodeNarc-varning (onödig null-check) och synkade det kvarvarande hårdkodade "id = 1" i validateIntegrityDetectsTamperedAuditRow med idOfRow-hjälparen, enligt granskningens förslag. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
AuditLogService.recordIntegrityRemediation(companyId, auditLogId, reason): documents that a specific audit-log row's hash mismatch is a known, already-explained historical anomaly (e.g. data corrupted by a bug that's since been fixed) - without ever touching the broken row'sentry_hash/previous_hash. It appends a new, normally-chainedINTEGRITY_REMEDIATIONentry pointing at the row it explains.validateIntegrity()/validateIntegrity(companyId)now exclude documented rows from the critical list (so they stop blockingReportIntegrityService-gated operations), whilelistDocumentedExceptions(companyId)surfaces them separately for transparency. Structural chain-head problems always stay critical, since they can't be tied to (and explained away for) one specific row.AGENTS.mdcodifying the rule this whole thing exists because of: never patch ledger tables directly with SQL to fix bad data, corrections go through the app's own domain operations (same append-only pattern as correction vouchers), and a discovered hash-integrity violation gets documented via this new method - never by rewriting hashes or by callingrebuildIntegrityChain/repairIntegrityForAllCompaniesas general-purpose repair tools (those are one-time, migration-gated fixes for specific historical bugs, not a mechanism to silence future violations).Test plan
./gradlew build- full suite green (compile, CodeNarc, Spotless, all tests)AuditLogServiceTest: remediating a genuinely broken row moves it from critical to documented and the chain still validates; remediating a healthy row is rejected; remediating an unknown row id is rejected; a blank reason is rejected🤖 Generated with Claude Code