fix: thirteenth-round audit (duplicate-key data loss, corrupt-file UX, docs)

- Editing a price/tax-rate to a key (effective date + amount) that already exists silently
  dropped it via a no-op TreeSet.add. addPrice/addTaxRate now return whether the add landed;
  the edit panels detect a collision, roll back to the prior values, and warn instead of
  persisting a silent deletion.
- Corrupt data file is no longer a dead end: load() moves the unreadable file aside to a
  unique .corrupt-<timestamp> sibling and throws actionable recovery guidance (and the next
  launch starts cleanly from the seed). Documented the recovery procedure in README.
- README no longer attributes find-sec-bugs to the security workflow; it runs in the mvn
  verify gate (CI), matching SECURITY.md.

Build green: 64 tests, JaCoCo gate, SpotBugs + find-sec-bugs 0; gitleaks/Trivy/Semgrep clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: Milton Adina Shisia

README.md

@@ -119,6 +119,11 @@ java -Dpos.data.file=/path/to/store.csv -jar target/point-of-sale-1.0.0.jar
 Application logs are written to a rolling file at `~/.pos/logs/pos-N.log` (so diagnostics survive even
 when the app is launched by double-clicking the jar, with no console attached).
 
+**Recovering from a corrupt data file.** Rather than silently overwriting damaged data, the app refuses
+to start on an unreadable data file. It moves the bad file aside to `…/StoreData_v2024FA.csv.corrupt-<timestamp>`
+and reports where, then starts fresh from the bundled seed on the next launch. To restore older data,
+replace the data file with a known-good copy (e.g. a renamed `.corrupt-…` backup).
+
 ## Demo credentials
 
 At the login screen:
@@ -183,10 +188,10 @@ masked to the last four digits and never written to disk. The bundled data is en
 this project is an educational demo that must not handle real cardholder data.
 
 Security is verified empirically, not assumed: a dedicated [security workflow](.github/workflows/security.yml)
-runs **CodeQL**, **Semgrep**, **Trivy** + **OWASP Dependency-Check**, **gitleaks**, and **find-sec-bugs**
-(also a `mvn verify` gate) on every push, with **Dependabot** watching dependencies — currently a clean,
-zero-finding pass. Full details, the threat model, and how to report a vulnerability are in
-[SECURITY.md](SECURITY.md).
+runs **CodeQL**, **Semgrep**, **Trivy** + **OWASP Dependency-Check**, and **gitleaks** on every push,
+complemented by **find-sec-bugs** in the `mvn verify` build gate (CI) and **Dependabot** watching
+dependencies — currently a clean, zero-finding pass. Full details, the threat model, and how to report a
+vulnerability are in [SECURITY.md](SECURITY.md).
 
 ## License
 

src/main/java/POSDM/CsvStoreRepository.java

@@ -103,25 +103,60 @@ public static CsvStoreRepository defaultRepository() {
     public Store load() {
         Store store = new Store();
         boolean fromDataFile = Files.exists(dataFile);
+        boolean corrupt;
         try (BufferedReader reader = openForRead()) {
             if (reader == null) {
                 LOG.info("No store data file or seed resource found; starting with an empty store");
                 return store;
             }
             parse(reader, store);
-            if (fromDataFile && isEffectivelyEmpty(store) && Files.size(dataFile) > 0) {
-                // A present, non-empty file that yields no usable records is corrupt. Refuse to
-                // return a silently-empty store, which a later save would overwrite irrecoverably.
-                throw new StorePersistenceException(
-                        "Store data file is present but could not be parsed (possibly corrupt): "
-                                + dataFile);
-            }
+            // A present, non-empty file that yields no usable records is corrupt.
+            corrupt = fromDataFile && isEffectivelyEmpty(store) && Files.size(dataFile) > 0;
         } catch (IOException e) {
             throw new StorePersistenceException("Failed to read store data from " + dataFile, e);
         }
+        if (corrupt) {
+            // Refuse to return a silently-empty store (a later save would overwrite the file
+            // irrecoverably). The reader is now closed, so move the bad file aside — the next
+            // launch
+            // then starts cleanly from the seed without the user touching a hidden dotfile — and
+            // fail with actionable guidance rather than a bare path.
+            Path quarantined = quarantineCorruptFile();
+            String guidance =
+                    quarantined != null
+                            ? " It has been moved to "
+                                    + quarantined
+                                    + ", so the application will start fresh on the next launch. To"
+                                    + " restore older data, replace "
+                                    + dataFile
+                                    + " with a known-good copy."
+                            : " Rename or remove "
+                                    + dataFile
+                                    + " to start fresh, or restore a known-good backup.";
+            throw new StorePersistenceException(
+                    "Your saved data file could not be read and may be damaged." + guidance);
+        }
         return store;
     }
 
+    /**
+     * Best-effort move of a corrupt data file to a unique {@code .corrupt-<timestamp>} sibling.
+     *
+     * @return the new path, or {@code null} if it could not be moved (logged)
+     */
+    private Path quarantineCorruptFile() {
+        try {
+            String stamp =
+                    LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss"));
+            Path target = dataFile.resolveSibling(dataFile.getFileName() + ".corrupt-" + stamp);
+            Files.move(dataFile, target);
+            return target;
+        } catch (IOException | RuntimeException e) {
+            LOG.log(Level.WARNING, "Could not move aside the corrupt data file " + dataFile, e);
+            return null;
+        }
+    }
+
     private static boolean isEffectivelyEmpty(Store store) {
         return store.getName().isEmpty()
                 && store.getItems().isEmpty()

src/main/java/POSPD/Item.java

@@ -91,10 +91,12 @@ public void setTaxCategory(String taxCategory, Store store) {
      * Adds a price to this item's price history.
      *
      * @param price the price to add
+     * @return {@code true} if it was added; {@code false} if an equal price (same effective date
+     *     and amount) is already present, in which case the set is unchanged
      */
-    public void addPrice(Price price) {
+    public boolean addPrice(Price price) {
         price.setItem(this);
-        prices.add(price);
+        return prices.add(price);
     }
 
     /**

src/main/java/POSPD/TaxCategory.java

@@ -64,9 +64,11 @@ public BigDecimal getTaxRateForDate(LocalDate date) {
      * Adds a tax rate to this category.
      *
      * @param taxRate the rate to add
+     * @return {@code true} if it was added; {@code false} if an equal rate (same effective date and
+     *     rate) is already present, in which case the set is unchanged
      */
-    public void addTaxRate(TaxRate taxRate) {
-        taxRates.add(taxRate);
+    public boolean addTaxRate(TaxRate taxRate) {
+        return taxRates.add(taxRate);
     }
 
     /**

src/main/java/POSUI/PriceEditPanel.java

@@ -131,6 +131,15 @@ public void actionPerformed(ActionEvent e) {
                                     JOptionPane.WARNING_MESSAGE);
                             return;
                         }
+                        // Capture the current values so the edit can roll back if it would collide
+                        // with an existing price (a TreeSet add is a no-op on an equal key).
+                        BigDecimal origAmount = price.getPrice();
+                        LocalDate origEffective = price.getEffectiveDate();
+                        LocalDate origEnd =
+                                price instanceof PromoPrice
+                                        ? ((PromoPrice) price).getEndDate()
+                                        : null;
+
                         // Remove before mutating so the Item's price TreeSet re-sorts.
                         if (!isAdd) {
                             item.removePrice(price);
@@ -140,7 +149,23 @@ public void actionPerformed(ActionEvent e) {
                         if (price instanceof PromoPrice) {
                             ((PromoPrice) price).setEndDate(end);
                         }
-                        item.addPrice(price);
+                        if (!item.addPrice(price)) {
+                            // An equal price already exists; roll back so nothing is silently lost.
+                            price.setPrice(origAmount);
+                            price.setEffectiveDate(origEffective);
+                            if (price instanceof PromoPrice) {
+                                ((PromoPrice) price).setEndDate(origEnd);
+                            }
+                            if (!isAdd) {
+                                item.addPrice(price);
+                            }
+                            JOptionPane.showMessageDialog(
+                                    PriceEditPanel.this,
+                                    "A price with that effective date and amount already exists.",
+                                    "Invalid input",
+                                    JOptionPane.WARNING_MESSAGE);
+                            return;
+                        }
                         if (!SaveSupport.saveOrWarn(null, storeService)) {
                             return;
                         }

src/main/java/POSUI/TaxRateEditPanel.java

@@ -74,13 +74,31 @@ public void actionPerformed(ActionEvent e) {
                                     JOptionPane.WARNING_MESSAGE);
                             return;
                         }
+                        // Capture current values so the edit can roll back if it would collide with
+                        // an existing rate (a TreeSet add is a no-op on an equal key).
+                        BigDecimal origRate = taxRate.getTaxRate();
+                        LocalDate origEffective = taxRate.getEffectiveDate();
+
                         // Remove before mutating so the TreeSet re-sorts on the new effective date.
                         if (!isAdd) {
                             taxCategory.removeTaxRate(taxRate);
                         }
                         taxRate.setTaxRate(rate);
                         taxRate.setEffectiveDate(effective);
-                        taxCategory.addTaxRate(taxRate);
+                        if (!taxCategory.addTaxRate(taxRate)) {
+                            // An equal rate already exists; roll back so nothing is silently lost.
+                            taxRate.setTaxRate(origRate);
+                            taxRate.setEffectiveDate(origEffective);
+                            if (!isAdd) {
+                                taxCategory.addTaxRate(taxRate);
+                            }
+                            JOptionPane.showMessageDialog(
+                                    TaxRateEditPanel.this,
+                                    "A rate with that effective date and rate already exists.",
+                                    "Invalid input",
+                                    JOptionPane.WARNING_MESSAGE);
+                            return;
+                        }
                         if (!SaveSupport.saveOrWarn(null, storeService)) {
                             return;
                         }

src/test/java/POSDM/PersistenceRoundTripTest.java

@@ -259,6 +259,14 @@ void corruptFileRejected(@TempDir Path dir) throws IOException {
         Path file = dir.resolve("store.csv");
         Files.writeString(file, "  garbage binary\nnot a known record type\n");
         assertThrows(StorePersistenceException.class, () -> new CsvStoreRepository(file).load());
+        // The corrupt file is quarantined (moved aside) so the next launch starts cleanly.
+        assertFalse(Files.exists(file), "corrupt file should be moved aside");
+        try (java.util.stream.Stream<Path> entries = Files.list(dir)) {
+            assertTrue(
+                    entries.anyMatch(
+                            p -> p.getFileName().toString().startsWith("store.csv.corrupt-")),
+                    "a .corrupt- backup should remain");
+        }
     }
 
     @Test

src/test/java/POSPD/PriceOrderingTest.java

@@ -1,7 +1,11 @@
 package POSPD;
 
 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 java.math.BigDecimal;
+import java.time.LocalDate;
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
 
@@ -37,4 +41,21 @@ void identicalPricesCollapse() {
         item.addPrice(new Price("2.00", "1/1/24"));
         assertEquals(1, item.getPrices().size());
     }
+
+    @Test
+    @DisplayName("addPrice/addTaxRate report a duplicate key instead of silently dropping it")
+    void addReportsDuplicate() {
+        Item item = new Item("1", "Widget");
+        assertTrue(item.addPrice(new Price("2.00", "1/1/24")));
+        assertFalse(
+                item.addPrice(new Price("2.00", "1/1/24")), "equal price must report not-added");
+        assertEquals(1, item.getPrices().size());
+
+        TaxCategory food = new TaxCategory("Food");
+        assertTrue(food.addTaxRate(new TaxRate(LocalDate.of(2024, 1, 1), new BigDecimal("0.07"))));
+        assertFalse(
+                food.addTaxRate(new TaxRate(LocalDate.of(2024, 1, 1), new BigDecimal("0.07"))),
+                "equal rate must report not-added");
+        assertEquals(1, food.getTaxRates().size());
+    }
 }