An UPDATE on encrypted columns wrote nothing for years, and three bugs took turns hiding it

·5 mins

A desktop app maintained an activity history in SQLite, but the UI froze on the first entry into any state. An action could start as ‘in progress’ and later complete, yet the UI would keep showing ‘in progress’ forever.

The update function looked fine at first glance, but nobody had actually run it under test, and it had 0% test coverage. That made the bug easy to miss and hard to trust.

The setup

The app encrypts the path columns as it writes them (column level encryption, not the whole database as with SQLCipher; see why fields rather than the file ). With a random IV per row, you can’t query the encrypted columns at all. To make rows matchable, derive the IV from the plaintext so the same path always encrypts to the same ciphertext. You store the IV alongside the row, base64 encoded:

history(id, run_id, path_enc, type, state, timestamp, iv)

You can’t put a plaintext path in a WHERE clause against path_enc. So the update does three steps:

  1. Check the row’s IV.
  2. Re-encrypt the current path with that IV, then SELECT to check whether the row still matches by id, run, path, and type.
  3. Run an UPDATE with the same match predicate.

Reproduce with the real class

One test that stores, updates, and reloads through the real DAO, with an in-memory schema and a dummy key:

TEST_F(HistoryDaoTest, update_StoredUpload_LoadShowsNewState) {
    HistoryDao dao(&_database);
    Action upload { action(Action::Upload, Action::IN_PROGRESS) };
    upload.rowId = dao.store(&upload, 5);
    upload.status = Action::OK;

    dao.update(&upload, 5);

    EXPECT_EQ(load().value(0).state(), Action::OK);
}

It failed. The reloaded state was still IN_PROGRESS, which was precisely where the problem was.

Three faults, each hiding the next

QByteArray iv = queryIv.value(0).toByteArray();            // 1: still base64
...
query.prepare("SELECT id FROM history"                     // 2: no space before WHERE
              "WHERE id = :id AND path_enc = :path ...");
bool ok = database->exec(&queryIv);                        // 2: runs the IV query again
...
QSqlQuery update_query(db);
update_query.prepare("UPDATE history SET state = :state ... WHERE <same predicate>");
...
database->exec(&query);                                    // 3: runs the SELECT, not the UPDATE
database->commit();
return true;
  1. The IV was never decoded. store() decodes the base64 IV before encryption, but update() passed the IV through still encoded. The encrypt helper rejected an IV of the wrong length and threw, so every call stopped there. Before the length check was introduced, a wrong IV would have produced a ciphertext that was impossible to match. In either case, nothing was ever written. The bug had been there since the commit that introduced encryption, because the two sibling functions in that commit disagreed.
  2. The guard SELECT never ran. A later refactor concatenated two string literals with no space between them (historyWHERE), so prepare() failed. The failure was only logged. Then exec() ran on the IV query, which succeeded. A follow-up “fix typo in query” commit fixed half of the SQL and left the missing space.
  3. The UPDATE never ran. A rename to update_query missed the exec() call, which kept executing query instead.

Nothing ever got past fault 1, so faults 2 and 3 were never executed in production. Read the code as a chain of what would have happened if each fault had been fixed one at a time: fix the IV, and the function returns true while writing nothing; fix the SELECT as well, and fault 3 reruns a successful SELECT, commits, and returns true. Each fix would look like progress, and the result would be a function that reports success and writes nothing.

Date the bug before you blame the refactor

The first suspect was the 2022 refactor that added the second SELECT. Running git log -S on the decode call and reading store() and update() side by side in the encryption commit moved the start date back to 2019. That matters for the fix: the refactor wasn’t the thing to revert, and the original rule from before encryption was still the spec.

The tempting shortcut

The obvious simplification is to drop the IV lookup and the re-encryption and run UPDATE … WHERE id = :id. That passes the test above.

It also deletes the rule. The version from before encryption updated a row only while its id, run, paths, and type all still matched. Every later commit kept that predicate. Updating by id alone means a stale action can overwrite whatever row now sits behind that id. So the fix was one small change per fault, keeping the predicate:

QByteArray iv = QByteArray::fromBase64(queryIv.value(0).toByteArray(),
                                       QByteArray::Base64UrlEncoding);
query.prepare("SELECT id FROM history "
              "WHERE id = :id AND path_enc = :path ...");
bool ok = database->exec(&query);
...
database->exec(&update_query);

A second test pins the rule:

TEST_F(HistoryDaoTest, update_OtherPath_LeavesTheRowAlone) {
    HistoryDao dao(&_database);
    Action upload { action(Action::Upload, Action::IN_PROGRESS) };
    upload.rowId = dao.store(&upload, 5);
    upload.name = "other.jpg";          // same id, different path
    upload.status = Action::OK;

    dao.update(&upload, 5);

    EXPECT_EQ(load().value(0).state(), Action::IN_PROGRESS);
}

Before the fix, this test passed too, trivially, because nothing was ever updated. A negative test proves something only once its positive twin is green. After the fix, both tests passed, and so did the full suite (787 tests).

Side notes

  • QSqlQuery::size() returns -1 on SQLite, because the driver doesn’t report query sizes. A if (query.size() > 1) guard never fires there. Count with while (query.next()), or use SELECT COUNT(*).
  • Deriving the IV from the plaintext makes the encryption deterministic: equal paths give equal ciphertexts, which is what makes matching possible. With AES-GCM, never reuse that IV for a different plaintext. If you need equality lookups on encrypted columns, consider a misuse-resistant mode (AES-SIV) or a separate blind-index column (an HMAC of the plaintext) instead.

Takeaway

  • Test a write path as store → change → reload through the real class against a real database. “Returns true” and “rows changed” are different claims.
  • Check every prepare() and exec() result and stop on failure. A failure that’s only logged lets the function carry on and commit.
  • Use one QSqlQuery variable per statement and never pass the wrong one to exec(). After an UPDATE, check numRowsAffected(); zero rows is a valid SQL result, not an error.
  • When you fix a chain like this, expect the next fault to surface. Keep the reproducing test red until the whole round trip works.
  • Before simplifying a WHERE clause, find out why each condition is there. Pin the rule with a negative test.