From ef5c096e847cdf0229d1bf00c0424b88d4beaedc Mon Sep 17 00:00:00 2001 From: James Maes Date: Fri, 16 Oct 2026 08:42:05 -0500 Subject: [PATCH] fix: resolve virtual fields and keep the access-log worker off unfinished instances - SecuredDataClassifications.effective looked fields up with QTableMetaData.getField, which throws for a virtual (computed) field, so any user read of a table with one failed (kof22's permalink sitePath and siteUrl on every content table, and API validation). Virtual fields now take the table classification. - The access-log worker opened a QContext on every tick, even with an empty queue. QContext validates an unvalidated instance, and the worker starts while the host is still building it, so the first tick validated (and enriched) a half-built instance on another thread; one failure cancelled every later tick for good. Idle ticks now return before opening a context, and a failed tick is logged and retried. Co-Authored-By: Claude Opus 5.5 --- .../secureddata/SecuredDataAccessLogger.java | 27 +++++++++++++++++- .../SecuredDataClassifications.java | 6 +++- .../SecuredDataAccessLoggerTest.java | 28 +++++++++++++++++++ .../secureddata/SecuredDataResolverTest.java | 19 +++++++++++++ 4 files changed, 78 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/kingsrook/qbits/secureddata/SecuredDataAccessLogger.java b/src/main/java/com/kingsrook/qbits/secureddata/SecuredDataAccessLogger.java index 0e9c806..ed77c5f 100644 --- a/src/main/java/com/kingsrook/qbits/secureddata/SecuredDataAccessLogger.java +++ b/src/main/java/com/kingsrook/qbits/secureddata/SecuredDataAccessLogger.java @@ -104,7 +104,7 @@ public class SecuredDataAccessLogger implements AutoCloseable return (thread); }); long interval = runtime.config().getAccessLogFlushIntervalMillis(); - worker.scheduleWithFixedDelay(this::flush, interval, interval, TimeUnit.MILLISECONDS); + worker.scheduleWithFixedDelay(this::scheduledFlush, interval, interval, TimeUnit.MILLISECONDS); shutdownHook = new Thread(this::close, "secured-data-access-log-shutdown"); Runtime.getRuntime().addShutdownHook(shutdownHook); } @@ -332,13 +332,38 @@ public class SecuredDataAccessLogger implements AutoCloseable + /******************************************************************************* + ** The worker's tick. An exception would cancel every later run of a + ** scheduled task, so a failed drain is logged (value-free) and retried on + ** the next tick; the rows stay queued. + *******************************************************************************/ + void scheduledFlush() + { + try + { + flush(); + } + catch(RuntimeException e) + { + LOG.warn("Unable to drain the secured-data access log; retrying on the next tick"); + } + } + + + /******************************************************************************* ** Drain safely both from the worker and explicit test/host shutdown calls. + ** An empty queue opens no context: QContext validates an unvalidated + ** instance, and the worker starts while the host is still building it. *******************************************************************************/ public void flush() { synchronized(flushLock) { + if(queue.isEmpty()) + { + return; + } QContext.withTemporaryContext(new CapturedContext(instance, new QSystemUserSession(instance)), () -> { PendingLog pending; diff --git a/src/main/java/com/kingsrook/qbits/secureddata/SecuredDataClassifications.java b/src/main/java/com/kingsrook/qbits/secureddata/SecuredDataClassifications.java index 6347380..7c8b178 100644 --- a/src/main/java/com/kingsrook/qbits/secureddata/SecuredDataClassifications.java +++ b/src/main/java/com/kingsrook/qbits/secureddata/SecuredDataClassifications.java @@ -54,7 +54,11 @@ public final class SecuredDataClassifications { return (DataClassification.valueOf(row.getValueString("classification"))); } - QFieldMetaData field = table.getField(fieldName); + //////////////////////////////////////////////////////////////////// + // getFields, not getField: a virtual field is not in the map and // + // getField throws, while it still takes the table classification // + //////////////////////////////////////////////////////////////////// + QFieldMetaData field = table.getFields() == null || fieldName == null ? null : table.getFields().get(fieldName); DataClassification classification = field == null ? null : SecuredDataMetadata.classification(table, field); return (classification == null ? SecuredDataMetadata.classification(table, null) : classification); } diff --git a/src/test/java/com/kingsrook/qbits/secureddata/SecuredDataAccessLoggerTest.java b/src/test/java/com/kingsrook/qbits/secureddata/SecuredDataAccessLoggerTest.java index 9e5d1a3..76f2d30 100644 --- a/src/test/java/com/kingsrook/qbits/secureddata/SecuredDataAccessLoggerTest.java +++ b/src/test/java/com/kingsrook/qbits/secureddata/SecuredDataAccessLoggerTest.java @@ -46,6 +46,7 @@ import com.kingsrook.qqq.backend.core.exceptions.QPermissionDeniedException; import com.kingsrook.qqq.backend.core.model.data.QRecord; import com.kingsrook.qqq.backend.core.model.metadata.QInstance; import com.kingsrook.qqq.backend.core.model.metadata.code.QCodeReference; +import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData; import com.kingsrook.qqq.backend.core.utils.JsonUtils; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; @@ -359,4 +360,31 @@ class SecuredDataAccessLoggerTest { } } + + + /******************************************************************************* + ** The worker starts while the host is still building its instance. An idle + ** tick opens no context, so it never validates (or enriches) an unfinished + ** instance on the worker thread, and a failing tick is logged instead of + ** cancelling every later tick. + *******************************************************************************/ + @Test + void idleTicksNeverValidateAnUnfinishedInstance() throws Exception + { + setup(); + SecuredDataRuntime runtime = SecuredDataRuntime.current(); + QInstance unfinished = SecuredDataTestSupport.base(); + unfinished.addTable(new QTableMetaData().withName("unfinished").withBackendName("not_registered_yet")); + SecuredDataAccessLogger idle = new SecuredDataAccessLogger(unfinished, runtime); + try + { + idle.flush(); + idle.scheduledFlush(); + assertThat(unfinished.getHasBeenValidated()).isFalse(); + } + finally + { + idle.close(); + } + } } diff --git a/src/test/java/com/kingsrook/qbits/secureddata/SecuredDataResolverTest.java b/src/test/java/com/kingsrook/qbits/secureddata/SecuredDataResolverTest.java index be767dc..1d66bf0 100644 --- a/src/test/java/com/kingsrook/qbits/secureddata/SecuredDataResolverTest.java +++ b/src/test/java/com/kingsrook/qbits/secureddata/SecuredDataResolverTest.java @@ -33,6 +33,7 @@ import com.kingsrook.qqq.backend.core.context.QContext; import com.kingsrook.qqq.backend.core.model.data.QRecord; import com.kingsrook.qqq.backend.core.model.metadata.fields.QFieldMetaData; import com.kingsrook.qqq.backend.core.model.metadata.fields.QFieldType; +import com.kingsrook.qqq.backend.core.model.metadata.fields.QVirtualFieldMetaData; import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -326,4 +327,22 @@ class SecuredDataResolverTest CLOCK.now = NOW.plusSeconds(10); assertThat(resolver.isRevealActive("person", "email", 7)).isFalse(); } + + + /******************************************************************************* + ** A virtual (computed) field is not in getFields, and getField throws for + ** it: it resolves through the table classification instead of failing the + ** whole read. + *******************************************************************************/ + @Test + void virtualFieldsTakeTheTableClassification() throws Exception + { + QVirtualFieldMetaData display = new QVirtualFieldMetaData("displayName", QFieldType.STRING); + person.withVirtualFields(List.of(display)); + assertThat(SecuredDataClassifications.effective(person, "displayName")).isNull(); + assertThat(resolver.resolve(person, display).tier()).isEqualTo(FieldAccessTier.FULL); + person.withSupplementalMetaData(new AiTableMetaData().withClassification(DataClassification.PII)); + assertThat(SecuredDataClassifications.effective(person, "displayName")).isEqualTo(DataClassification.PII); + assertThat(resolver.resolve(person, display).tier()).isEqualTo(FieldAccessTier.MASKED); + } } -- 2.54.0