Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public ResponseEntity<?> analyzeQuery(
httpRequest.getHeader(HttpHeaders.AUTHORIZATION)
),
accessControlService.getCurrentUsername(),
accessControlService.isCurrentUserAdmin(),
accessControlService.currentUserMayMutateSql(),
Boolean.TRUE.equals(request.getMutationConfirmed())
),
dbType
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public ResponseEntity<?> executeReadOnlyQuery(@RequestBody McpReadOnlyQueryReque
queryRequest,
QueryExecutionContext.mcp(
accessControlService.getCurrentUsername(),
accessControlService.isCurrentUserAdmin()
accessControlService.currentUserMayMutateSql()
)
);
return ResponseEntity.ok(Map.of(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ private QueryExecutionContext queryExecutionContext(QueryRequest queryRequest, H
return QueryExecutionContext.forSqlSurface(
McpTokenService.isMcpAuthorizationHeader(httpRequest.getHeader(HttpHeaders.AUTHORIZATION)),
accessControlService.getCurrentUsername(),
accessControlService.isCurrentUserAdmin(),
accessControlService.currentUserMayMutateSql(),
Boolean.TRUE.equals(queryRequest.getMutationConfirmed())
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,20 @@
import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContextHolder;

/**
* Execution origin + mutation privileges for a SQL run.
*
* <p>{@code actorMayMutate} is true for built-in ADMIN and DBA (wired from
* {@code AccessControlService.currentUserMayMutateSql()}). It gates Editor/MCP
* DDL/DML; chat stays {@link MutationMode#READ_ONLY_ONLY}. The same flag is
* consulted by data-access policy during query execution so mutators are not
* blocked by schema redaction meant for read-only roles.
*/
public record QueryExecutionContext(
QueryExecutionOrigin origin,
MutationMode mutationMode,
String actorUsername,
boolean actorIsAdmin,
boolean actorMayMutate,
boolean mutationConfirmed
) {

Expand All @@ -27,12 +36,12 @@ public static QueryExecutionContext chat() {
);
}

public static QueryExecutionContext editor(String actorUsername, boolean actorIsAdmin, boolean mutationConfirmed) {
public static QueryExecutionContext editor(String actorUsername, boolean actorMayMutate, boolean mutationConfirmed) {
return new QueryExecutionContext(
QueryExecutionOrigin.EDITOR,
actorIsAdmin ? MutationMode.MAY_MUTATE : MutationMode.READ_ONLY_ONLY,
actorMayMutate ? MutationMode.MAY_MUTATE : MutationMode.READ_ONLY_ONLY,
actorUsername,
actorIsAdmin,
actorMayMutate,
mutationConfirmed
);
}
Expand All @@ -51,39 +60,39 @@ public static QueryExecutionContext mcp(String actorUsername) {
return mcp(actorUsername, false);
}

public static QueryExecutionContext mcp(String actorUsername, boolean actorIsAdmin) {
return mcp(actorUsername, actorIsAdmin, false);
public static QueryExecutionContext mcp(String actorUsername, boolean actorMayMutate) {
return mcp(actorUsername, actorMayMutate, false);
}

/**
* MCP / coding-agent SQL. Developers stay read-only. Admins may run
* MCP / coding-agent SQL. Developers stay read-only. Admins and DBAs may run
* non-destructive DDL/DML after the same confirmation gate as the Editor.
* DROP and TRUNCATE stay blocked in {@link QueryExecutionPolicyService}.
*/
public static QueryExecutionContext mcp(
String actorUsername,
boolean actorIsAdmin,
boolean actorMayMutate,
boolean mutationConfirmed
) {
return new QueryExecutionContext(
QueryExecutionOrigin.MCP,
actorIsAdmin ? MutationMode.MAY_MUTATE : MutationMode.READ_ONLY_ONLY,
actorMayMutate ? MutationMode.MAY_MUTATE : MutationMode.READ_ONLY_ONLY,
actorUsername,
actorIsAdmin,
actorMayMutate,
mutationConfirmed
);
}

public static QueryExecutionContext forSqlSurface(
boolean mcpBearer,
String actorUsername,
boolean actorIsAdmin,
boolean actorMayMutate,
boolean mutationConfirmed
) {
if (mcpBearer) {
return mcp(actorUsername, actorIsAdmin, mutationConfirmed);
return mcp(actorUsername, actorMayMutate, mutationConfirmed);
}
return editor(actorUsername, actorIsAdmin, mutationConfirmed);
return editor(actorUsername, actorMayMutate, mutationConfirmed);
}

public static QueryExecutionContext scheduled() {
Expand All @@ -100,12 +109,16 @@ public static QueryExecutionContext api(String actorUsername) {
return api(actorUsername, false);
}

public static QueryExecutionContext api(String actorUsername, boolean actorIsAdmin) {
/**
* API / dashboard SQL is always read-only. The {@code actorMayMutate} flag here
* only influences data-access policy bypass (admins), not mutation mode.
*/
public static QueryExecutionContext api(String actorUsername, boolean actorMayMutate) {
return new QueryExecutionContext(
QueryExecutionOrigin.API,
MutationMode.READ_ONLY_ONLY,
actorUsername,
actorIsAdmin,
actorMayMutate,
false
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public static QueryExecutionPolicyException editorMutationForbidden(String query
return new QueryExecutionPolicyException(
EDITOR_MUTATION_FORBIDDEN,
HttpStatus.FORBIDDEN,
"Only admins can execute DDL or DML from the SQL Editor. This Editor run was blocked before any database changes were attempted.",
"Only admins or DBAs can execute DDL or DML from the SQL Editor. This Editor run was blocked before any database changes were attempted.",
false,
queryType,
List.of()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,15 +156,15 @@ public PolicyDecision enforce(
}

StatementClassification mutation = classifications.getFirst();
if (!effectiveContext.actorIsAdmin()) {
if (!effectiveContext.actorMayMutate()) {
throw QueryExecutionPolicyException.editorMutationForbidden(mutation.queryType());
}

if (origin == QueryExecutionOrigin.MCP
&& isDropOrTruncateStatement(mutation.queryType(), statements.getFirst())) {
throw QueryExecutionPolicyException.unsafeMutation(
"DROP and TRUNCATE are blocked on MCP and coding-agent loops. "
+ "CREATE, ALTER, and DML still require admin privileges plus confirmation.",
+ "CREATE, ALTER, and DML still require admin or DBA privileges plus confirmation.",
mutation.queryType()
);
}
Expand Down Expand Up @@ -316,7 +316,7 @@ private StatementClassification classifyStatement(String statement, QueryExecuti
// provider's `isReadOnlyQuery` strips only comments, still sees the leading quote,
// and answers false. That combination used to fall through as mutating=true, and a
// user pasting a SELECT with the double quotes it carried in source code was told
// "Only admins can execute DDL or DML" — a permissions error for a syntax problem.
// "Only admins or DBAs can execute DDL or DML" — a permissions error for a syntax problem.
//
// It stays blocked: the parser rejected it, so nothing here can vouch for it being
// read-only, and this is deliberately reported the same way to admins rather than
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ public QueryGuardDecision enforcePreExecution(
ConnectionChatAccessPolicyService.EffectivePolicy policy = policyService.resolveEffectivePolicy(
connectionId,
executionContext.actorUsername(),
executionContext.actorIsAdmin()
executionContext.actorMayMutate()
);
if (!policy.protectsAnything()) {
return QueryGuardDecision.allow(policy);
Expand Down Expand Up @@ -339,7 +339,7 @@ public QueryResult redactResult(
ConnectionChatAccessPolicyService.EffectivePolicy policy = policyService.resolveEffectivePolicy(
connectionId,
executionContext.actorUsername(),
executionContext.actorIsAdmin()
executionContext.actorMayMutate()
);
if (!policy.protectsAnything() || !policy.redactMode()) {
return result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import com.dbaagent.model.ChatFeedback;
import com.dbaagent.model.EffectiveConnectionAccess;
import com.dbaagent.model.Permission;
import com.dbaagent.model.Role;
import com.dbaagent.repository.AnalysisHistoryRepository;
import com.dbaagent.repository.ChatFeedbackRepository;
import com.dbaagent.repository.ChatRepository;
Expand Down Expand Up @@ -297,6 +298,38 @@ public boolean isCurrentUserAdmin() {
.anyMatch(authority -> "ROLE_ADMIN".equals(authority.getAuthority()));
}

/**
* Whether the current principal may run confirmed DDL/DML on SQL surfaces
* (Editor / MCP). Built-in ADMIN and DBA only — not custom roles, and not
* DEVELOPER / DATA_ENGINEER. Distinct from {@link #isCurrentUserAdmin()}:
* DBA must not receive MANAGE_USERS or other admin-only product controls.
*/
public boolean currentUserMayMutateSql() {
if (ImpersonationContext.isActive()) {
return ImpersonationContext.current()
.map(state -> {
if (state.target() == null) {
return false;
}
Role role = state.target().getRoleEnum();
return role == Role.ADMIN || role == Role.DBA;
})
.orElse(false);
}
if (!authEnabled) {
return true;
}
Authentication authentication = currentAuthentication();
if (authentication == null || !authentication.isAuthenticated()) {
return false;
}
return authentication.getAuthorities().stream()
.anyMatch(authority -> {
String value = authority.getAuthority();
return "ROLE_ADMIN".equals(value) || "ROLE_DBA".equals(value);
});
}

private Authentication currentAuthentication() {
return SecurityContextHolder.getContext().getAuthentication();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ void useAnalyzeTrue_mcpBearer_usesMcpExecutionContext() {
when(httpRequest.getHeader(HttpHeaders.AUTHORIZATION))
.thenReturn("Bearer dsql_mcp_public.secret");
when(accessControlService.getCurrentUsername()).thenReturn("admin");
when(accessControlService.isCurrentUserAdmin()).thenReturn(true);
when(accessControlService.currentUserMayMutateSql()).thenReturn(true);
when(explainPlanService.analyzeQuery(eq("conn-1"), anyString(), eq(true)))
.thenReturn(new ExplainPlanAnalysis());

Expand All @@ -154,7 +154,7 @@ void useAnalyzeTrue_mcpBearer_usesMcpExecutionContext() {
assertThat(captor.getValue().origin()).isEqualTo(QueryExecutionOrigin.MCP);
assertThat(captor.getValue().mutationMode())
.isEqualTo(QueryExecutionContext.MutationMode.MAY_MUTATE);
assertThat(captor.getValue().actorIsAdmin()).isTrue();
assertThat(captor.getValue().actorMayMutate()).isTrue();
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,29 @@ void mcpFactoryProducesReadOnlyContextWithMcpOrigin() {
assertThat(ctx.origin()).isEqualTo(QueryExecutionOrigin.MCP);
assertThat(ctx.mutationMode()).isEqualTo(QueryExecutionContext.MutationMode.READ_ONLY_ONLY);
assertThat(ctx.actorUsername()).isEqualTo("user-1");
assertThat(ctx.actorIsAdmin()).isFalse();
assertThat(ctx.actorMayMutate()).isFalse();
assertThat(ctx.mutationConfirmed()).isFalse();
}

@Test
void mcpFactoryHonoursAdminFlagFromSecurityContext() {
void mcpFactoryHonoursMayMutateFlagFromSecurityContext() {
QueryExecutionContext ctx = QueryExecutionContext.mcp("admin", true);
assertThat(ctx.origin()).isEqualTo(QueryExecutionOrigin.MCP);
assertThat(ctx.actorUsername()).isEqualTo("admin");
assertThat(ctx.actorIsAdmin()).isTrue();
assertThat(ctx.actorMayMutate()).isTrue();
assertThat(ctx.mutationMode()).isEqualTo(QueryExecutionContext.MutationMode.MAY_MUTATE);
assertThat(ctx.mutationConfirmed()).isFalse();
}

@Test
void mcpDbaMayMutateWithConfirmation() {
QueryExecutionContext ctx = QueryExecutionContext.mcp("dba", true, true);
assertThat(ctx.origin()).isEqualTo(QueryExecutionOrigin.MCP);
assertThat(ctx.mutationMode()).isEqualTo(QueryExecutionContext.MutationMode.MAY_MUTATE);
assertThat(ctx.actorMayMutate()).isTrue();
assertThat(ctx.mutationConfirmed()).isTrue();
}

@Test
void mcpAdminConfirmedFactoryPassesConfirmationFlag() {
QueryExecutionContext ctx = QueryExecutionContext.mcp("admin", true, true);
Expand All @@ -36,11 +45,19 @@ void mcpAdminConfirmedFactoryPassesConfirmationFlag() {
}

@Test
void mcpNonAdminRemainsReadOnlyEvenWhenConfirmed() {
void mcpNonMutatorRemainsReadOnlyEvenWhenConfirmed() {
QueryExecutionContext ctx = QueryExecutionContext.mcp("dev", false, true);
assertThat(ctx.origin()).isEqualTo(QueryExecutionOrigin.MCP);
assertThat(ctx.mutationMode()).isEqualTo(QueryExecutionContext.MutationMode.READ_ONLY_ONLY);
assertThat(ctx.actorIsAdmin()).isFalse();
assertThat(ctx.actorMayMutate()).isFalse();
}

@Test
void editorDbaGetsMayMutateMode() {
QueryExecutionContext ctx = QueryExecutionContext.editor("dba", true, false);
assertThat(ctx.origin()).isEqualTo(QueryExecutionOrigin.EDITOR);
assertThat(ctx.mutationMode()).isEqualTo(QueryExecutionContext.MutationMode.MAY_MUTATE);
assertThat(ctx.actorMayMutate()).isTrue();
}

@Test
Expand All @@ -60,7 +77,7 @@ void scheduledFactoryProducesMayMutateInternalActor() {
assertThat(ctx.origin()).isEqualTo(QueryExecutionOrigin.SCHEDULED);
assertThat(ctx.mutationMode()).isEqualTo(QueryExecutionContext.MutationMode.MAY_MUTATE);
assertThat(ctx.actorUsername()).isNull();
assertThat(ctx.actorIsAdmin()).isTrue();
assertThat(ctx.actorMayMutate()).isTrue();
assertThat(ctx.mutationConfirmed()).isTrue();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,48 @@ void editorMutation_adminRequiresConfirmation() {
assertThat(exception.getWarnings()).isNotEmpty();
}

@Test
void editorMutation_dbaRequiresConfirmation() {
QueryExecutionPolicyException exception = assertThrows(
QueryExecutionPolicyException.class,
() -> service.enforce(
new QueryRequest("UPDATE customers SET property_status = 'ACTIVE' WHERE customer_id = 9", null, null),
QueryExecutionContext.editor("dba", true, false),
"mysql"
)
);

assertThat(exception.getErrorCode()).isEqualTo(QueryExecutionPolicyException.EDITOR_MUTATION_CONFIRMATION_REQUIRED);
assertThat(exception.isRequiresConfirmation()).isTrue();
}

@Test
void editorMutation_dbaConfirmedIsAllowed() {
QueryExecutionPolicyService.PolicyDecision decision = service.enforce(
new QueryRequest("UPDATE customers SET property_status = 'ACTIVE' WHERE customer_id = 9", null, null),
QueryExecutionContext.editor("dba", true, true),
"mysql"
);

assertThat(decision.mutating()).isTrue();
assertThat(decision.primaryQueryType()).isEqualTo("UPDATE");
}

@Test
void editorMutation_developerStillForbidden() {
QueryExecutionPolicyException exception = assertThrows(
QueryExecutionPolicyException.class,
() -> service.enforce(
new QueryRequest("DELETE FROM bookings WHERE id = 1", null, null),
QueryExecutionContext.editor("developer", false, true),
"mysql"
)
);

assertThat(exception.getErrorCode()).isEqualTo(QueryExecutionPolicyException.EDITOR_MUTATION_FORBIDDEN);
assertThat(exception.getMessage()).contains("admins or DBAs");
}

@Test
void editorConfirmedDeleteWithoutWhere_isBlocked() {
QueryExecutionPolicyException exception = assertThrows(
Expand Down Expand Up @@ -619,7 +661,7 @@ void mcpAdminExplainDrop_isBlockedEvenWhenConfirmed() {
// ── A malformed statement is a syntax error, not a permissions problem ──────────
//
// Reported from the field: a user pasted a SELECT that still carried the double
// quotes it had in source code and was told "Only admins can execute DDL or DML from
// quotes it had in source code and was told "Only admins or DBAs can execute DDL or DML from
// the SQL Editor", which reads as a permissions problem and sent them looking for a
// role fix. The statement is neither DDL nor DML — it is not valid SQL at all.
//
Expand All @@ -644,7 +686,7 @@ void selectPastedWithItsSurroundingQuotes_isReportedAsASyntaxErrorNotAPermission
);

assertThat(exception.getErrorCode()).isEqualTo(QueryExecutionPolicyException.STATEMENT_NOT_PARSEABLE);
assertThat(exception.getMessage()).doesNotContain("Only admins");
assertThat(exception.getMessage()).doesNotContain("Only admins or DBAs");
assertThat(exception.getMessage()).contains("could not parse");
}

Expand Down
Loading
Loading