#1553 - gate history + currentQuery egress on /ai/ask at send-time - #1585
Merged
Conversation
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
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.
Fixes #1553.
Problem
The
/ai/askegress gate (#1554) stripped only thecellsetDigest.historyandcurrentQuerystill reached the LLM ungated, so cell-derived data leaked even atschema-only. Killer case: a turn-1aggregatedinsight is replayed by the client at turn-2schema-only— the assistant turn carrying model-emitted figures egresses regardless of the current posture. History is client-supplied and untrusted every turn, so the only enforceable control is at send-time.Fix (Option B — "drop, not redact", send-time re-gate)
When the active posture forbids aggregated egress (
!egressPermitsCellData(), fail-closed on a null/unwired guard), at the request-building seam — before theNlAskRequestis constructed, so both providers are covered in one place:emit_insight/emit_email_draftfigures.currentQueryfilter members — clearfilters[].memberswhile keeping the query structure (measures/rows/columns/cube + filter dimension/hierarchy/level refs). The model still sees the query shape, not which members were picked.Applied in both
askInternal(the/ai/askseam) and theaskChainedpreamble, mirroring the existingcellsetDigeststrip and its fail-closed semantics. Both re-gates are copy-not-mutate: history is filtered into a new list;currentQueryis redacted on a mapper round-trip copy, so the caller-owned objects are never touched.The agentic
toolTranscriptneeded no change — its egress-withheld path (resultDigest == null) already holds, and underschema-onlythe chained loop stops before it ever executes/digests, so no data-bearing tool turn is produced. Confirmed with a test.Tests
Added to
AiAskServiceTest, mirroring the #1554 egress/digest-strip tests:schema-only: assistant history turns dropped, user turns kept;currentQuery.filters[].memberscleared, structure retained.aggregated: history + currentQuery pass through untouched.history/currentQuery.toolTranscriptwithheld-path stays withheld underschema-only.One pre-existing test (
passesQuestionAndHistoryToProviderUnchanged) asserted full-history passthrough with no guard wired; it now wires anAGGREGATEDposture to keep asserting the plumbing forwards history intact (the fail-closed drop is covered by the dedicated tests).mvn -o -pl saiku-core/saiku-service test -Dtest=AiAskServiceTest,OssieAiAskServiceTest,AiPolicyGuardTest,ToolTurnTest→ 103 tests, 0 failures. spotless:apply clean.