diff --git a/app/src/main/groovy/se/alipsa/accounting/service/AuditLogService.groovy b/app/src/main/groovy/se/alipsa/accounting/service/AuditLogService.groovy index fe1269c..ef42173 100644 --- a/app/src/main/groovy/se/alipsa/accounting/service/AuditLogService.groovy +++ b/app/src/main/groovy/se/alipsa/accounting/service/AuditLogService.groovy @@ -157,8 +157,15 @@ final class AuditLogService { private static List validateIntegrityForCompany(Sql sql, long companyId) { List problems = [] + // Archiving a fiscal year (see FiscalYearReplacementService.archiveFiscalYearAuditLogRows) + // resets the chain head to the latest *live* row so the next insert skips over the rows + // being archived, rather than chaining off them. So the row immediately following an + // archival deliberately does not continue the raw by-id sequence; it jumps back to the + // last live row instead. expectedPreviousHash tracks the raw sequence (what a row chains + // from under normal, non-archival inserts); expectedLastLiveHash tracks that skip target. String expectedPreviousHash = null - String actualLastHash = null + String expectedLastLiveHash = null + String actualLastLiveHash = null sql.rows(''' select id, event_type as eventType, @@ -172,13 +179,15 @@ final class AuditLogService { details, previous_hash as previousHash, entry_hash as entryHash, - created_at as createdAt + created_at as createdAt, + archived from audit_log where company_id = ? order by id ''', [companyId]).each { GroovyRowResult row -> AuditLogEntry entry = mapEntry(row) - if (entry.previousHash != expectedPreviousHash) { + boolean archived = row.get('archived') as Boolean + if (entry.previousHash != expectedPreviousHash && entry.previousHash != expectedLastLiveHash) { problems << ("Auditlogg ${entry.id} har fel föregående hash." as String) } String calculated = calculateHash(new AuditEntrySeed( @@ -198,12 +207,15 @@ final class AuditLogService { problems << ("Auditlogg ${entry.id} har ogiltig hash." as String) } expectedPreviousHash = entry.entryHash - actualLastHash = entry.entryHash + if (!archived) { + expectedLastLiveHash = entry.entryHash + actualLastLiveHash = entry.entryHash + } } AuditChainHead chainHead = loadChainHead(sql, companyId) if (chainHead == null) { problems << 'Auditloggkedjans huvud saknas.' - } else if (chainHead.lastEntryHash != actualLastHash) { + } else if (chainHead.lastEntryHash != actualLastLiveHash) { problems << 'Auditloggkedjans huvud pekar inte på sista auditraden.' } problems @@ -451,14 +463,23 @@ final class AuditLogService { @PackageScope static void rebuildIntegrityChain(Sql sql, long companyId) { - String previousHash = null + // Mirrors the live write path (see validateIntegrityForCompany): a live row chains from + // the latest *live* row, skipping over any archived rows in between - the same skip that + // archiveFiscalYearAuditLogRows()'s chain-head reset produces on insert. An archived row + // chains from whichever row immediately precedes it, since that is what it actually + // chained from back when it was itself still live. Collapsing both cases onto a single + // "previous row by id" chain (as an earlier version of this method did) makes a live row + // downstream of an archived block chain through it, which validateIntegrityForCompany + // does not treat as equivalent to a real chain-head-reset skip. + String lastLiveHash = null sql.withBatch(''' update audit_log set previous_hash = ?, entry_hash = ? where id = ? ''') { groovy.sql.BatchingPreparedStatementWrapper statement -> - String batchPreviousHash = null + String rawPreviousHash = null + String liveHash = null sql.rows(''' select id, event_type as eventType, @@ -470,11 +491,14 @@ final class AuditLogService { actor, summary, details, - created_at as createdAt + created_at as createdAt, + archived from audit_log where company_id = ? order by id ''', [companyId]).each { GroovyRowResult row -> + boolean archived = row.get('archived') as Boolean + String previousHash = archived ? rawPreviousHash : liveHash String entryHash = calculateHash(new AuditEntrySeed( eventType: row.get('eventType') as String, voucherId: longOrNull(row.get('voucherId')), @@ -485,15 +509,94 @@ final class AuditLogService { actor: row.get('actor') as String, summary: row.get('summary') as String, details: SqlValueMapper.toClob(row.get('details')), - previousHash: batchPreviousHash, + previousHash: previousHash, createdAt: SqlValueMapper.toLocalDateTime(row.get('createdAt')) )) - statement.addBatch([batchPreviousHash, entryHash, row.get('id')]) - batchPreviousHash = entryHash + statement.addBatch([previousHash, entryHash, row.get('id')]) + rawPreviousHash = entryHash + if (!archived) { + liveHash = entryHash + } + } + lastLiveHash = liveHash + } + updateChainHead(sql, companyId, lastLiveHash) + } + + /** + * One-time repair for databases affected by the pre-V27 archival bug (see + * V27__audit_log_decouple_references.sql): archiving used to null out all five reference + * columns without recalculating entry_hash. Best-effort restores whatever is recoverable + * from each row's own (untouched) details text, then rebuilds the hash chain for any + * company that still fails validation afterwards - closing out rows where the original + * value isn't recoverable (e.g. CANCEL_VOUCHER/CORRECTION_VOUCHER never recorded + * fiscalYearId in details in the first place). + */ + @PackageScope + static void repairIntegrityForAllCompanies(Sql sql) { + repairArchivedReferencesFromDetails(sql) + sql.rows('select distinct company_id as companyId from audit_log').each { GroovyRowResult row -> + long companyId = ((Number) row.get('companyId')).longValue() + if (!validateIntegrityForCompany(sql, companyId).isEmpty()) { + rebuildIntegrityChain(sql, companyId) + } + } + } + + @PackageScope + static void repairArchivedReferencesFromDetails(Sql sql) { + sql.rows(''' + select id, + voucher_id as voucherId, + attachment_id as attachmentId, + fiscal_year_id as fiscalYearId, + accounting_period_id as accountingPeriodId, + vat_period_id as vatPeriodId, + details + from audit_log + where archived = true + and details is not null + and (voucher_id is null + or attachment_id is null + or fiscal_year_id is null + or accounting_period_id is null + or vat_period_id is null) + ''').each { GroovyRowResult row -> + Map parsed = parseDetails(SqlValueMapper.toClob(row.get('details'))) + if (parsed.isEmpty()) { + return + } + // transferVoucherId is VAT_PERIOD_LOCKED's name for what recordEvent() stored as + // voucher_id - see recordVatPeriodLocked(). + Long voucherId = longOrNull(row.get('voucherId')) ?: longOrNull(parsed.voucherId) ?: longOrNull(parsed.transferVoucherId) + Long attachmentId = longOrNull(row.get('attachmentId')) ?: longOrNull(parsed.attachmentId) + Long fiscalYearId = longOrNull(row.get('fiscalYearId')) ?: longOrNull(parsed.fiscalYearId) + Long accountingPeriodId = longOrNull(row.get('accountingPeriodId')) ?: longOrNull(parsed.accountingPeriodId) + Long vatPeriodId = longOrNull(row.get('vatPeriodId')) ?: longOrNull(parsed.vatPeriodId) + sql.executeUpdate(''' + update audit_log + set voucher_id = ?, + attachment_id = ?, + fiscal_year_id = ?, + accounting_period_id = ?, + vat_period_id = ? + where id = ? + ''', [voucherId, attachmentId, fiscalYearId, accountingPeriodId, vatPeriodId, row.get('id')]) + } + } + + private static Map parseDetails(String details) { + Map parsed = [:] + if (!details) { + return parsed + } + details.split('\n').each { String line -> + int separatorIndex = line.indexOf('=') + if (separatorIndex > 0) { + parsed[line.substring(0, separatorIndex)] = line.substring(separatorIndex + 1) } - previousHash = batchPreviousHash } - updateChainHead(sql, companyId, previousHash) + parsed } private AuditLogEntry recordStandaloneEvent(String eventType, String summary, String details, long companyId) { diff --git a/app/src/main/groovy/se/alipsa/accounting/service/DatabaseService.groovy b/app/src/main/groovy/se/alipsa/accounting/service/DatabaseService.groovy index a79c5a6..c9f49ac 100644 --- a/app/src/main/groovy/se/alipsa/accounting/service/DatabaseService.groovy +++ b/app/src/main/groovy/se/alipsa/accounting/service/DatabaseService.groovy @@ -50,7 +50,8 @@ final class DatabaseService { new MigrationDefinition(23, 'V23__attachment_status.sql', '/db/migrations/V23__attachment_status.sql'), new MigrationDefinition(24, 'V24__voucher_navigation_index.sql', '/db/migrations/V24__voucher_navigation_index.sql'), new MigrationDefinition(25, 'V25__company_accounting_method.sql', '/db/migrations/V25__company_accounting_method.sql'), - new MigrationDefinition(26, 'V26__accounting_instructions.sql', '/db/migrations/V26__accounting_instructions.sql') + new MigrationDefinition(26, 'V26__accounting_instructions.sql', '/db/migrations/V26__accounting_instructions.sql'), + new MigrationDefinition(27, 'V27__audit_log_decouple_references.sql', '/db/migrations/V27__audit_log_decouple_references.sql') ] static DatabaseService newForTesting() { @@ -86,8 +87,16 @@ final class DatabaseService { AppPaths.ensureDirectoryStructure() withTransaction { Sql sql -> ensureSchemaVersionTable(sql) - applyPendingMigrations(sql) + List appliedVersions = applyPendingMigrations(sql) applyIndexes(sql) + // V27 stopped the archival bug that nulled out audit_log's reference columns without + // recalculating entry_hash, but couldn't itself repair rows already corrupted by it. + // Runs exactly once, right when a database first crosses V27, never again afterwards - + // this must stay a one-time migration step, not a general "if hash invalid, fix it" + // runtime behavior, or it would silently mask real tampering after this point too. + if (appliedVersions.contains(27)) { + AuditLogService.repairIntegrityForAllCompanies(sql) + } } log.info("Database initialized at ${AppPaths.databaseBasePath()}.mv.db") } @@ -178,7 +187,7 @@ final class DatabaseService { ((Number) row.total).intValue() == 1 } - private void applyPendingMigrations(Sql sql) { + private List applyPendingMigrations(Sql sql) { int currentVersion = currentSchemaVersion(sql) List pending = MIGRATIONS.findAll { MigrationDefinition migration -> migration.version > currentVersion @@ -194,6 +203,7 @@ final class DatabaseService { ) log.info("Applied migration ${migration.name}.") } + pending.collect { MigrationDefinition migration -> migration.version } } private int currentSchemaVersion(Sql sql) { diff --git a/app/src/main/groovy/se/alipsa/accounting/service/FiscalYearReplacementService.groovy b/app/src/main/groovy/se/alipsa/accounting/service/FiscalYearReplacementService.groovy index 17a54af..5d40ab9 100644 --- a/app/src/main/groovy/se/alipsa/accounting/service/FiscalYearReplacementService.groovy +++ b/app/src/main/groovy/se/alipsa/accounting/service/FiscalYearReplacementService.groovy @@ -160,14 +160,14 @@ final class FiscalYearReplacementService { @PackageScope static void archiveFiscalYearAuditLogRows(Sql sql, long companyId, long fiscalYearId) { + // Reference columns are deliberately left untouched: entry_hash was computed from them + // at insert time and must never change afterwards, since later rows chain off it via + // previous_hash. audit_log no longer has restrictive foreign keys to voucher/attachment/ + // fiscal_year/accounting_period/vat_period (see V27), so these rows are free to keep + // pointing at records that the subsequent purge deletes. sql.executeUpdate(''' update audit_log - set archived = true, - fiscal_year_id = null, - accounting_period_id = null, - vat_period_id = null, - voucher_id = null, - attachment_id = null + set archived = true where company_id = ? and archived = false and ( diff --git a/app/src/main/groovy/se/alipsa/accounting/service/UpdateService.groovy b/app/src/main/groovy/se/alipsa/accounting/service/UpdateService.groovy index 532cc8b..173ef06 100644 --- a/app/src/main/groovy/se/alipsa/accounting/service/UpdateService.groovy +++ b/app/src/main/groovy/se/alipsa/accounting/service/UpdateService.groovy @@ -9,6 +9,7 @@ import se.alipsa.accounting.support.LoggingConfigurer import java.net.http.HttpClient import java.net.http.HttpRequest import java.net.http.HttpResponse +import java.nio.charset.StandardCharsets import java.nio.file.Files import java.nio.file.Path import java.nio.file.StandardCopyOption @@ -257,6 +258,9 @@ final class UpdateService { Path backupDir = installDir.resolve('.update-backup') String newMainJar = mainJarFileName(extractedDir) ?: '' String newVersion = versionFromMainJar(newMainJar) ?: '' + Map configUpdates = newMainJar.isEmpty() + ? [:] + : prepareUpdatedConfigs(installDir, extractedDir, stagingDir, newVersion) UpdaterScriptContext context = new UpdaterScriptContext( stagingDir, extractedDir, @@ -265,7 +269,8 @@ final class UpdateService { backupDir, launcherCommand, newMainJar, - newVersion + newVersion, + configUpdates ) Path script @@ -281,13 +286,18 @@ final class UpdateService { } private static String windowsUpdaterScript(UpdaterScriptContext context) { + // Redirection is applied to a single ( ... ) block rather than via "call :main >> log" + // followed by a separate "exit /b" line. The call/label form requires cmd.exe to seek + // back into this file to read the line after the call once :main returns; if anything + // (antivirus scanning, disk latency) makes the file briefly unreadable at that exact + // moment, cmd.exe fails with "The batch file cannot be found" and drops into an + // interactive prompt instead of exiting, leaving a stray window behind. Wrapping the + // whole body in one redirected block avoids that second read entirely - once the block + // finishes there is nothing left to seek back for. """\ @echo off set "LOG_FILE=${context.updaterLog}" -call :main >> "%LOG_FILE%" 2>&1 -exit /b %ERRORLEVEL% - -:main +( echo [%DATE% %TIME%] Starting update. echo [%DATE% %TIME%] Install dir: ${context.installDir} echo [%DATE% %TIME%] Extracted dir: ${context.extractedDir} @@ -312,20 +322,23 @@ if errorlevel 1 ( for %%f in ("${context.backupDir}\\*.jar") do move "%%f" "${context.installDir}\\" rd /s /q "${context.backupDir}" echo [%DATE% %TIME%] Update failed. Please try again. - pause exit /b 1 ) echo [%DATE% %TIME%] Updating launcher configuration. -${context.newMainJar.isEmpty() ? '' : windowsConfigUpdateCommand(context.installDir, context.newMainJar, context.newVersion)} -echo [%DATE% %TIME%] Cleaning update staging files. +${windowsConfigCopyCommands(context.configUpdates)}echo [%DATE% %TIME%] Cleaning update staging files. rd /s /q "${context.backupDir}" rd /s /q "${context.extractedDir}" del "${context.stagingDir}\\*.zip" +del "${context.stagingDir}\\*.cfg.new" 2>nul echo [%DATE% %TIME%] Launching application. ${context.launcherCommand.isEmpty() ? 'echo Update complete.' : "start \"\" ${context.launcherCommand}"} echo [%DATE% %TIME%] Update script finished. -rem Keep this script until the next update. Deleting it here prevents cmd.exe -rem from returning from the call to :main and leaves a "batch file cannot be found" window. +) >> "%LOG_FILE%" 2>&1 +rem Unlike the Unix script, this one deliberately does not delete itself (no "del %~f0"). +rem cmd.exe reads a running batch file from disk as it goes; deleting it out from under +rem itself risks the same "file cannot be found" failure the block redirection above +rem already works around. The leftover updater.bat in the staging directory is expected - +rem the next update overwrites it in place. """.stripIndent() } @@ -367,11 +380,11 @@ if ! cp "${context.extractedDir}/"*.jar "${context.installDir}/"; then fail "Update failed. Please try again." fi log "Updating launcher configuration." -${context.newMainJar.isEmpty() ? '' : unixConfigUpdateCommand(context.installDir, context.newMainJar, context.newVersion)} -log "Cleaning update staging files." +${unixConfigCopyCommands(context.configUpdates)}log "Cleaning update staging files." rm -rf "${context.backupDir}" rm -rf "${context.extractedDir}" rm -f "${context.stagingDir}/"*.zip +rm -f "${context.stagingDir}/"*.cfg.new log "Launching application." ${context.launcherCommand.isEmpty() ? 'echo "Update complete."' : "${context.launcherCommand} &"} log "Update script finished." @@ -398,29 +411,87 @@ rm -f "\$0" matcher.matches() ? matcher.group(1) : null } - private static String unixConfigUpdateCommand(Path installDir, String newMainJar, String newVersion) { - String versionCommand = newVersion.isEmpty() - ? '' - : " sed -i.bak \"s#^java-options=-Djpackage.app-version=.*#java-options=-Djpackage.app-version=${newVersion}#\" \"\$cfg\"\n" - """\ -for cfg in "${installDir}/"*.cfg; do - [ -f "\$cfg" ] || continue - sed -i.bak "s#^app.classpath=\\\$APPDIR/app-.*\\.jar#app.classpath=\\\$APPDIR/${newMainJar}#" "\$cfg" -${versionCommand} rm -f "\$cfg.bak" -done -""".stripIndent() + /** + * Regenerates each *.cfg file's app.classpath block from the jars actually present in + * the extracted update, rather than only patching the main app-*.jar entry. Dependency + * jars whose filename changes between releases (e.g. a version bump) would otherwise be + * left as dangling classpath entries pointing at a jar the update just deleted. + */ + private static Map prepareUpdatedConfigs(Path installDir, Path extractedDir, Path stagingDir, String newVersion) { + Map updates = [:] + if (!Files.isDirectory(installDir) || !Files.isDirectory(extractedDir)) { + return updates + } + List newJarNames = jarFileNames(extractedDir) + if (newJarNames.isEmpty()) { + return updates + } + List cfgFiles + Files.list(installDir).withCloseable { stream -> + cfgFiles = stream.findAll { Path path -> + Files.isRegularFile(path) && path.fileName.toString().endsWith('.cfg') + } + } + cfgFiles.each { Path cfgPath -> + List lines = Files.readAllLines(cfgPath, StandardCharsets.UTF_8) + List updatedLines = [] + boolean classpathInserted = false + lines.each { String line -> + if (line.startsWith('app.classpath=')) { + if (!classpathInserted) { + newJarNames.each { String jarName -> updatedLines << "app.classpath=\$APPDIR/${jarName}".toString() } + classpathInserted = true + } + return + } + if (!newVersion.isEmpty() && line.startsWith('java-options=-Djpackage.app-version=')) { + updatedLines << "java-options=-Djpackage.app-version=${newVersion}".toString() + return + } + updatedLines << line + } + Path newCfgPath = stagingDir.resolve("${cfgPath.fileName}.new") + Files.write(newCfgPath, updatedLines, StandardCharsets.UTF_8) + updates[cfgPath] = newCfgPath + } + updates } - private static String windowsConfigUpdateCommand(Path installDir, String newMainJar, String newVersion) { - String mainJarUpdateCommand = - " \$lines = Get-Content -LiteralPath \$cfg.FullName; \$updatedLines = \$lines | ForEach-Object { if (\$_.StartsWith('app.classpath=') -and \$_ -match '[\\\\/]app-') { 'app.classpath=\$APPDIR/${newMainJar}' } else { \$_ } }; \$updatedLines | Set-Content -LiteralPath \$cfg.FullName" - String escapedInstallDir = installDir.toString().replace('\\', '\\\\') - String versionCommand = newVersion.isEmpty() - ? '' - : " (Get-Content -LiteralPath \$cfg.FullName) -replace '^java-options=-Djpackage.app-version=.*', 'java-options=-Djpackage.app-version=${newVersion}' | Set-Content -LiteralPath \$cfg.FullName\n" - """\ -powershell -NoProfile -ExecutionPolicy Bypass -Command "Get-ChildItem -LiteralPath '${escapedInstallDir}' -Filter '*.cfg' | ForEach-Object { \$cfg = \$_; ${mainJarUpdateCommand}; ${versionCommand.trim()} }" -""".stripIndent() + private static List jarFileNames(Path directory) { + if (!Files.isDirectory(directory)) { + return [] + } + List names + Files.list(directory).withCloseable { stream -> + names = stream + .map { Path path -> path.fileName.toString() } + .findAll { String fileName -> fileName.endsWith('.jar') } + } + names.sort(String.CASE_INSENSITIVE_ORDER) + String mainJar = names.find { String name -> name ==~ /app-\d+(\.\d+)*\.jar/ } + if (mainJar != null) { + names.remove(mainJar) + names.add(0, mainJar) + } + names + } + + private static String unixConfigCopyCommands(Map configUpdates) { + if (configUpdates.isEmpty()) { + return '' + } + configUpdates.collect { Path original, Path staged -> + "cp \"${staged}\" \"${original}\" || fail \"Failed to update launcher configuration ${original}.\"" + }.join('\n') + '\n' + } + + private static String windowsConfigCopyCommands(Map configUpdates) { + if (configUpdates.isEmpty()) { + return '' + } + configUpdates.collect { Path original, Path staged -> + "copy /y \"${staged}\" \"${original}\"\nif errorlevel 1 (\n echo [%DATE% %TIME%] Failed to update launcher configuration ${original}.\n exit /b 1\n)" + }.join('\n') + '\n' } private static void launchUpdaterAndExit(Path updaterScript) { @@ -502,6 +573,7 @@ powershell -NoProfile -ExecutionPolicy Bypass -Command "Get-ChildItem -LiteralPa final String launcherCommand final String newMainJar final String newVersion + final Map configUpdates private UpdaterScriptContext( Path stagingDir, @@ -511,7 +583,8 @@ powershell -NoProfile -ExecutionPolicy Bypass -Command "Get-ChildItem -LiteralPa Path backupDir, String launcherCommand, String newMainJar, - String newVersion) { + String newVersion, + Map configUpdates) { this.stagingDir = stagingDir this.extractedDir = extractedDir this.installDir = installDir @@ -520,6 +593,7 @@ powershell -NoProfile -ExecutionPolicy Bypass -Command "Get-ChildItem -LiteralPa this.launcherCommand = launcherCommand this.newMainJar = newMainJar this.newVersion = newVersion + this.configUpdates = configUpdates } } diff --git a/app/src/main/resources/db/migrations/V27__audit_log_decouple_references.sql b/app/src/main/resources/db/migrations/V27__audit_log_decouple_references.sql new file mode 100644 index 0000000..9590cb0 --- /dev/null +++ b/app/src/main/resources/db/migrations/V27__audit_log_decouple_references.sql @@ -0,0 +1,25 @@ +-- The audit log is an append-only trail: once a row is written, its entry_hash must +-- never change, since later rows chain off it via previous_hash. Fiscal-year replacement +-- (SIE reimport) archives audit rows tied to the purged year and used to null out their +-- voucher_id/attachment_id/fiscal_year_id/accounting_period_id/vat_period_id columns so the +-- subsequent hard deletes of voucher/attachment/fiscal_year rows wouldn't violate the +-- "on delete restrict" foreign keys below. But entry_hash was computed from those columns +-- at insert time and was never recalculated, so every archived row permanently failed +-- validateIntegrity() afterwards. Dropping the restrictive foreign keys lets archived rows +-- keep pointing at (now possibly deleted) records instead, so they no longer need to be +-- nulled out. +-- +-- Repairing rows already corrupted by the old nulling behavior happens in Groovy +-- (AuditLogService.repairIntegrityForAllCompanies, invoked from DatabaseService right after +-- this migration applies) rather than here: reconstructing the five reference columns needs +-- per-event-type knowledge of which key each was recorded under in the details text (they +-- don't all use the same key names - VAT_PERIOD_LOCKED's voucher_id, for instance, was +-- recorded as transferVoucherId), and not every field is recoverable that way. Whatever +-- can't be recovered is closed out by rebuilding that company's hash chain from its current +-- (already-archived, un-nullable-further) state, which is easier to get right and test as +-- Groovy than as a wall of per-field regexes. +alter table audit_log drop constraint if exists fk_audit_log_voucher; +alter table audit_log drop constraint if exists fk_audit_log_attachment; +alter table audit_log drop constraint if exists fk_audit_log_fiscal_year; +alter table audit_log drop constraint if exists fk_audit_log_period; +alter table audit_log drop constraint if exists fk_audit_log_vat_period; diff --git a/app/src/test/groovy/integration/se/alipsa/accounting/service/AuditLogServiceTest.groovy b/app/src/test/groovy/integration/se/alipsa/accounting/service/AuditLogServiceTest.groovy index 4ad5c9a..8783747 100644 --- a/app/src/test/groovy/integration/se/alipsa/accounting/service/AuditLogServiceTest.groovy +++ b/app/src/test/groovy/integration/se/alipsa/accounting/service/AuditLogServiceTest.groovy @@ -4,6 +4,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals import static org.junit.jupiter.api.Assertions.assertFalse import static org.junit.jupiter.api.Assertions.assertTrue +import groovy.sql.GroovyRowResult import groovy.sql.Sql import org.junit.jupiter.api.AfterEach @@ -70,6 +71,104 @@ class AuditLogServiceTest { assertTrue(problems.any { String problem -> problem.contains('ogiltig hash') }) } + @Test + void rebuildIntegrityChainMakesLiveEntriesSkipArchivedOnesInBetween() { + long companyId = CompanyService.LEGACY_COMPANY_ID + auditLogService.logImport('A - kommer förbli levande', 'seed=A') + auditLogService.logExport('B - arkiveras i efterhand', 'seed=B') + auditLogService.logBackup('C - kommer förbli levande', 'seed=C') + + databaseService.withTransaction { Sql sql -> + sql.executeUpdate("update audit_log set archived = true where summary like 'B -%'") + AuditLogService.rebuildIntegrityChain(sql, companyId) + } + + databaseService.withTransaction { Sql sql -> + String hashOfA = firstHashMatching(sql, "summary like 'A -%'", 'entry_hash') + String previousHashOfC = firstHashMatching(sql, "summary like 'C -%'", 'previous_hash') + assertEquals(hashOfA, previousHashOfC, + 'C must chain from A (the last live entry), skipping the now-archived B in between') + } + assertEquals([], auditLogService.validateIntegrity()) + } + + @Test + void repairIntegrityForAllCompaniesRestoresRecoverableReferencesAndClosesTheRest() { + long companyId = CompanyService.LEGACY_COMPANY_ID + auditLogService.logImport('Kedjeankare', 'seed=anchor') + + databaseService.withTransaction { Sql sql -> + // Simulates rows corrupted by the pre-V27 archival bug: every reference column was + // nulled out regardless of event type, without recalculating entry_hash. The actual + // hash values here don't matter - repairIntegrityForAllCompanies's rebuild step + // recomputes them from whatever ends up in each row afterwards. + insertCorruptedArchivedRow(sql, companyId, 'CREATE_VOUCHER', + 'accountingDate=2026-01-05\ndescription=Test\nfiscalYearId=5\nstatus=ACTIVE\nvoucherId=999') + insertCorruptedArchivedRow(sql, companyId, 'ATTACHMENT_ADDED', + 'attachmentId=42\ncontentType=text/plain\nfileSize=10\nvoucherId=999') + insertCorruptedArchivedRow(sql, companyId, 'LOCK_PERIOD', + 'accountingPeriodId=7\nfiscalYearId=5\nperiodName=Januari') + insertCorruptedArchivedRow(sql, companyId, 'VAT_PERIOD_LOCKED', + 'fiscalYearId=5\nperiodName=Q1\nstatus=LOCKED\ntransferVoucherId=888\nvatPeriodId=3') + // CANCEL_VOUCHER never records fiscalYearId in details (see recordVoucherCancelled) - + // fiscal_year_id is genuinely unrecoverable here and must be closed out by rebuilding + // instead, not left permanently invalid. + insertCorruptedArchivedRow(sql, companyId, 'CANCEL_VOUCHER', + 'accountingDate=2026-01-05\ndescription=Test\nstatus=CANCELLED\nvoucherId=999') + + AuditLogService.repairIntegrityForAllCompanies(sql) + } + + databaseService.withTransaction { Sql sql -> + assertReference(sql, 'CREATE_VOUCHER', 'voucher_id', 999L) + assertReference(sql, 'CREATE_VOUCHER', 'fiscal_year_id', 5L) + assertReference(sql, 'ATTACHMENT_ADDED', 'attachment_id', 42L) + assertReference(sql, 'ATTACHMENT_ADDED', 'voucher_id', 999L) + assertReference(sql, 'LOCK_PERIOD', 'accounting_period_id', 7L) + assertReference(sql, 'LOCK_PERIOD', 'fiscal_year_id', 5L) + assertReference(sql, 'VAT_PERIOD_LOCKED', 'vat_period_id', 3L) + assertReference(sql, 'VAT_PERIOD_LOCKED', 'fiscal_year_id', 5L) + assertReference(sql, 'VAT_PERIOD_LOCKED', 'voucher_id', 888L, + 'voucher_id must be recovered from transferVoucherId, the key name VAT_PERIOD_LOCKED actually uses') + assertReference(sql, 'CANCEL_VOUCHER', 'voucher_id', 999L) + assertReference(sql, 'CANCEL_VOUCHER', 'fiscal_year_id', null, + 'fiscal_year_id was never in this event type\'s details, so it must stay null') + } + + assertEquals([], auditLogService.validateIntegrity(), + 'Every archived row must validate again, including CANCEL_VOUCHER whose fiscal_year_id could not be recovered') + } + + private static void insertCorruptedArchivedRow(Sql sql, long companyId, String eventType, String details) { + sql.executeInsert(''' + insert into audit_log ( + event_type, voucher_id, attachment_id, fiscal_year_id, accounting_period_id, vat_period_id, + actor, summary, details, previous_hash, entry_hash, created_at, company_id, archived + ) values (?, null, null, null, null, null, 'desktop-app', ?, ?, 'placeholder', 'placeholder', + current_timestamp, ?, true) + ''', [eventType, "Arkiverad ${eventType}".toString(), details, companyId]) + } + + private static void assertReference(Sql sql, String eventType, String column, Long expected, String message = null) { + GroovyRowResult row = sql.firstRow( + "select ${column} as columnValue from audit_log where event_type = ?".toString(), [eventType] + ) as GroovyRowResult + Object actual = row.get('columnValue') + Long actualValue = actual == null ? null : ((Number) actual).longValue() + if (message != null) { + assertEquals(expected, actualValue, message) + } else { + assertEquals(expected, actualValue) + } + } + + private static String firstHashMatching(Sql sql, String whereClause, String column) { + GroovyRowResult row = sql.firstRow( + "select ${column} as columnValue from audit_log where ${whereClause}".toString() + ) as GroovyRowResult + row.get('columnValue') as String + } + private static void restoreProperty(String name, String value) { if (value == null) { System.clearProperty(name) diff --git a/app/src/test/groovy/integration/se/alipsa/accounting/service/SieImportExportServiceTest.groovy b/app/src/test/groovy/integration/se/alipsa/accounting/service/SieImportExportServiceTest.groovy index 8038520..a3322fa 100644 --- a/app/src/test/groovy/integration/se/alipsa/accounting/service/SieImportExportServiceTest.groovy +++ b/app/src/test/groovy/integration/se/alipsa/accounting/service/SieImportExportServiceTest.groovy @@ -232,7 +232,30 @@ class SieImportExportServiceTest { chainHead?.get('lastEntryHash') as String, 'Chain head must reference the latest live audit log entry after replacement' ) + + GroovyRowResult archivedVoucherRow = sql.firstRow(''' + select voucher_id as voucherId, fiscal_year_id as fiscalYearId + from audit_log + where company_id = ? + and archived = true + and event_type = ? + ''', [CompanyService.LEGACY_COMPANY_ID, AuditLogService.CREATE_VOUCHER]) as GroovyRowResult + assertNotNull(archivedVoucherRow, 'The pre-existing voucher should have produced an archived CREATE_VOUCHER row') + assertNotNull( + archivedVoucherRow.get('voucherId'), + 'Archiving must not null out voucher_id, or entry_hash (computed from it) can never be reproduced again' + ) + assertNotNull( + archivedVoucherRow.get('fiscalYearId'), + 'Archiving must not null out fiscal_year_id, or entry_hash (computed from it) can never be reproduced again' + ) } + + assertEquals( + [], + auditLogService.validateIntegrity(CompanyService.LEGACY_COMPANY_ID), + 'Archiving audit rows during fiscal-year replacement must not break hash-chain integrity' + ) } @Test diff --git a/app/src/test/groovy/unit/se/alipsa/accounting/service/UpdateServiceUpdaterScriptTest.groovy b/app/src/test/groovy/unit/se/alipsa/accounting/service/UpdateServiceUpdaterScriptTest.groovy index e3902ba..bb9fe54 100644 --- a/app/src/test/groovy/unit/se/alipsa/accounting/service/UpdateServiceUpdaterScriptTest.groovy +++ b/app/src/test/groovy/unit/se/alipsa/accounting/service/UpdateServiceUpdaterScriptTest.groovy @@ -53,7 +53,14 @@ final class UpdateServiceUpdaterScriptTest { assertTrue(content.contains('log "Update failed while copying files, restoring backup..."')) assertTrue(content.contains('log "Updating launcher configuration."')) assertTrue(content.contains('log "Launching application."')) - assertTrue(content.contains('app-1.4.0.jar')) + Path stagedCfgPath = fixture.stagingDir.resolve('app.cfg.new') + Path installedCfgPath = fixture.installDir.resolve('app.cfg') + assertTrue(content.contains("cp \"${stagedCfgPath}\" \"${installedCfgPath}\" || fail")) + + String stagedCfg = Files.readString(fixture.stagingDir.resolve('app.cfg.new')) + assertTrue(stagedCfg.contains('app.classpath=$APPDIR/app-1.4.0.jar')) + assertTrue(stagedCfg.contains('app.classpath=$APPDIR/widgets-1.1.2.jar')) + assertFalse(stagedCfg.contains('widgets-1.1.1.jar')) } @Test @@ -67,17 +74,31 @@ final class UpdateServiceUpdaterScriptTest { assertEquals('updater.bat', script.fileName.toString()) assertTrue(content.contains("set \"LOG_FILE=${fixture.updaterLog}\"")) - assertTrue(content.contains('call :main >> "%LOG_FILE%" 2>&1')) + assertTrue(content.contains(') >> "%LOG_FILE%" 2>&1')) + assertFalse( + content.contains('call :main'), + 'The call/:main/exit-after-return pattern is what causes the "batch file cannot ' + + 'be found" stray window - the whole body must be redirected as one block instead' + ) assertTrue(content.contains('echo [%DATE% %TIME%] Starting update.')) assertTrue(content.contains('echo [%DATE% %TIME%] Backing up current JAR files.')) assertTrue(content.contains('echo [%DATE% %TIME%] Copying updated JAR files.')) assertTrue(content.contains('Update failed while copying files, restoring backup')) assertTrue(content.contains('echo [%DATE% %TIME%] Updating launcher configuration.')) assertTrue(content.contains('echo [%DATE% %TIME%] Launching application.')) - assertTrue(content.contains('app-1.4.0.jar')) - assertTrue(content.contains("'app.classpath=\$APPDIR/app-1.4.0.jar'")) - assertTrue(content.contains("\$_.StartsWith('app.classpath=')")) - assertFalse(content.contains('del "%~f0"')) + Path stagedCfgPath = fixture.stagingDir.resolve('app.cfg.new') + Path installedCfgPath = fixture.installDir.resolve('app.cfg') + assertTrue(content.contains("copy /y \"${stagedCfgPath}\" \"${installedCfgPath}\"")) + assertFalse(content.contains('del "%~f0"')) + + // the regenerated config staged for copy must list every jar from the new + // release, not just the main app-*.jar (this is what broke in production: + // a dependency jar's version changed and its classpath entry went stale). + String stagedCfg = Files.readString(fixture.stagingDir.resolve('app.cfg.new')) + assertTrue(stagedCfg.contains('app.classpath=$APPDIR/app-1.4.0.jar')) + assertTrue(stagedCfg.contains('app.classpath=$APPDIR/widgets-1.1.2.jar')) + assertFalse(stagedCfg.contains('widgets-1.1.1.jar')) + assertTrue(stagedCfg.contains('java-options=-Djpackage.app-version=1.4.0')) } @Test @@ -103,10 +124,53 @@ final class UpdateServiceUpdaterScriptTest { assertEquals(0, process.exitValue()) assertFalse(Files.exists(fixture.installDir.resolve('app-1.2.0.jar'))) assertTrue(Files.isRegularFile(fixture.installDir.resolve('app-1.4.0.jar'))) + assertFalse(Files.exists(fixture.installDir.resolve('widgets-1.1.1.jar'))) + assertTrue(Files.isRegularFile(fixture.installDir.resolve('widgets-1.1.2.jar'))) assertFalse(Files.exists(script)) String cfg = Files.readString(fixture.installDir.resolve('app.cfg')) assertTrue(cfg.contains('app.classpath=$APPDIR/app-1.4.0.jar')) + assertTrue(cfg.contains('app.classpath=$APPDIR/widgets-1.1.2.jar')) + assertFalse(cfg.contains('widgets-1.1.1.jar')) + assertTrue(cfg.contains('java-options=-Djpackage.app-version=1.4.0')) + + String log = Files.readString(fixture.updaterLog) + assertTrue(log.contains('Starting update.')) + assertTrue(log.contains('Backing up current JAR files.')) + assertTrue(log.contains('Copying updated JAR files.')) + assertTrue(log.contains('Updating launcher configuration.')) + assertTrue(log.contains('Update script finished.')) + } + + @Test + void windowsUpdaterScriptUpdatesFakeInstallDirectoryAndLogsProgress() { + assumeTrue(File.separatorChar == (char) '\\', 'Windows updater execution is only verified on Windows hosts.') + + ScriptFixture fixture = createFixture() + System.setProperty('os.name', 'Windows 11') + Path script = new UpdateService().writeUpdaterScript( + fixture.stagingDir, fixture.extractedDir, fixture.installDir, fixture.updaterLog) + restoreProperty('os.name', previousOsName) + + Process process = new ProcessBuilder('cmd', '/c', script.toString()) + .directory(fixture.stagingDir.toFile()) + .start() + boolean finished = process.waitFor(15, TimeUnit.SECONDS) + if (!finished) { + process.destroyForcibly() + } + + assertTrue(finished, 'Updater script did not finish within the timeout.') + assertEquals(0, process.exitValue()) + assertFalse(Files.exists(fixture.installDir.resolve('app-1.2.0.jar'))) + assertTrue(Files.isRegularFile(fixture.installDir.resolve('app-1.4.0.jar'))) + assertFalse(Files.exists(fixture.installDir.resolve('widgets-1.1.1.jar'))) + assertTrue(Files.isRegularFile(fixture.installDir.resolve('widgets-1.1.2.jar'))) + + String cfg = Files.readString(fixture.installDir.resolve('app.cfg')) + assertTrue(cfg.contains('app.classpath=$APPDIR/app-1.4.0.jar')) + assertTrue(cfg.contains('app.classpath=$APPDIR/widgets-1.1.2.jar')) + assertFalse(cfg.contains('widgets-1.1.1.jar')) assertTrue(cfg.contains('java-options=-Djpackage.app-version=1.4.0')) String log = Files.readString(fixture.updaterLog) @@ -128,8 +192,14 @@ final class UpdateServiceUpdaterScriptTest { Files.createDirectories(updaterLog.parent) Files.writeString(extractedDir.resolve('app-1.4.0.jar'), 'new jar') Files.writeString(installDir.resolve('app-1.2.0.jar'), 'old jar') + // a dependency jar whose version also changed between releases - this is what the + // updater used to leave stale in app.cfg, since it only ever rewrote the main jar line. + Files.writeString(extractedDir.resolve('widgets-1.1.2.jar'), 'new dependency jar') + Files.writeString(installDir.resolve('widgets-1.1.1.jar'), 'old dependency jar') Files.writeString(installDir.resolve('app.cfg'), - 'app.classpath=$APPDIR/app-1.2.0.jar\njava-options=-Djpackage.app-version=1.2.0\n') + 'app.classpath=$APPDIR/app-1.2.0.jar\n' + + 'app.classpath=$APPDIR/widgets-1.1.1.jar\n' + + 'java-options=-Djpackage.app-version=1.2.0\n') new ScriptFixture(stagingDir, extractedDir, installDir, updaterLog) }