Skip to content

Commit 722c64e

Browse files
localstack-spiral[bot]spiralhovaescoclaude
authored
LAV-4493: ALTER USER implicit current-user targeting (#4681)
* LAV-4493: investigator: capture requires disposable-user authentication without proxy rewriting Dispatch 1 ended voluntarily blocked at 2026-10-14T06:52:35, not from exhaustion. Recovered implementer JSONL item_31 and driver.log agree. Guarded captures requested Implicit_User_63254ed5 on Standard and Implicit_User_19e34f11 on Enterprise, but both authenticated as SNAPSHOT_TESTING. Initial unguarded captures mutated QUERY_TAG on SNAPSHOT_TESTING and completed UNSET; those invalid captures and all implementation edits were reverted. Verified October 14, 2026: after git fetch origin, HEAD and origin/main both equal 2fd2e760a0459a48096d7524a715f9851e710d37; git status --porcelain and git diff origin/main --stat are empty. The parser still requires parse_identifier() in vendor/sqlparser/src/parser/alter.rs:153, so this is not already fixed. Independent read-only identity probes: SNOWFLAKE_USER=LAV4493_INVESTIGATOR_IDENTITY_PROBE SNOWFLAKE_ROLE=PUBLIC make snowsql SQL='SELECT CURRENT_USER(), CURRENT_ROLE(), CURRENT_ACCOUNT()' returned SNAPSHOT_TESTING | PUBLIC | OD47041. With SNOWFLAKE_ACCOUNT=FS76419 and SNOWFLAKE_HOST=$SNOWFLAKE_HOST2 the same probe returned SNAPSHOT_TESTING | PUBLIC | FS76419. No objects were created and no ALTER statements were executed during investigation. Repository contract docs/agentic-dev-workflow.md:1209 documents credential replacement and the dummy worker key. Current localstack/spiral source read through the permitted GitHub GET API confirms internal/proxy/snowflake.go:202 calls login.encode with the configured Snowflake user, and :372-374 overwrites LOGIN_NAME, AUTHENTICATOR and TOKEN. This is authentication rewriting, not a missing package or host-allowlist refusal. Remote-history/sibling discovery found the identical restriction in investigator note 0e9f5aae1 (LAV-4352) and human direct-Cloud capture precedent 6ff891a22 (LAV-3996). Neither supplies a worker-side authentication-preserving capture path; no owning proxy-fix issue was identified in available evidence. Live Linear lookup is unavailable. Changing only CURRENT_ROLE cannot satisfy authenticated-user isolation. Human action required: provide an authorized Cloud capture path preserving the disposable user's LOGIN_NAME and key-pair JWT, OR capture and commit the new disposable-user tests and snapshots from a direct-Cloud environment, then resume LAV-4493. Capture with TEST_TARGET=SNOWFLAKE_CLOUD and --snapshot-update; require SELECT CURRENT_USER() to equal the requested disposable identity before any SET/UNSET. Cover unnamed/named SET and UNSET, quoted identity and unchanged second user. Never reuse invalid SNAPSHOT_TESTING captures or mutate the credential-bearing production user. No implementation or waiver is supplied by this note. Co-authored-by: Przemek Denkiewicz <przemyslaw.denkiewicz@localstack.cloud> * LAV-4493: capture Cloud baselines for unnamed ALTER USER self-targeting A disposable key-pair user runs ALTER USER [<name>] SET/UNSET QUERY_TAG on itself, with a CURRENT_USER() guard before any mutation. Captured against real Snowflake from a direct-Cloud environment (the worker proxy substitutes the login identity): the unnamed form changes exactly what the named form does, only on the authenticated user; a bystander and the connecting user stay unchanged. The unnamed cases are strict xfail until the parser accepts an omitted target. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * LAV-4493: resolve unnamed ALTER USER to the authenticated user Preserve an omitted target as None in the parser and AST display, then resolve it at execution with the schema-qualified Snowflake CURRENT_USER() function. Explicit targets keep their existing literal-name resolution; lookahead preserves even action-keyword user names instead of confusing them with the omitted-target form. No PostgreSQL role is used for targeting. Enable the human-captured disposable-user QUERY_TAG SET/UNSET snapshots from b159f3ea7 without re-capturing or changing any snapshot. Both quoted and unquoted users leave the bystander and connecting user unchanged. SHOW/DESCRIBE projections and independently owned action implementations are unchanged. Extend the existing vendored parser test with omission/round-trip and keyword-name cases. Its existing exact named-user assertion now wraps the same Ident in Some to match the optional AST field; its expected identity and all compat test assertions are unchanged. Validation: - Before implementation, the two unnamed compat cases failed on QUERY_TAG parsing while both named cases passed. - make check: passed. - make test-vendored-sqlparser: passed (run explicitly because make check's committed vendor-diff gate skips changes not yet committed). - make test-compat PYTEST_PATH='tests/queries/access_control/test_alter_user.py tests/queries/access_control/test_desc_user.py' PYTEST_XDIST_ARGS='': 45 passed. Co-authored-by: Przemek Denkiewicz <przemyslaw.denkiewicz@localstack.cloud> ## Test matrix Unnamed SET / unquoted identity / success + unchanged other users -> tests/queries/access_control/test_alter_user.py::test_alter_user_self_target_query_tag[unnamed-unquoted] Unnamed UNSET / unquoted identity / reset to inherited empty default -> tests/queries/access_control/test_alter_user.py::test_alter_user_self_target_query_tag[unnamed-unquoted] Unnamed SET / quoted mixed-case identity / success + unchanged other users -> tests/queries/access_control/test_alter_user.py::test_alter_user_self_target_query_tag[unnamed-quoted] Unnamed UNSET / quoted mixed-case identity / reset to inherited empty default -> tests/queries/access_control/test_alter_user.py::test_alter_user_self_target_query_tag[unnamed-quoted] Named SET / unquoted identity / equivalent outcome and isolation -> tests/queries/access_control/test_alter_user.py::test_alter_user_self_target_query_tag[named-unquoted] Named UNSET / unquoted identity / equivalent reset and isolation -> tests/queries/access_control/test_alter_user.py::test_alter_user_self_target_query_tag[named-unquoted] Named SET / quoted mixed-case identity / equivalent outcome and isolation -> tests/queries/access_control/test_alter_user.py::test_alter_user_self_target_query_tag[named-quoted] Named UNSET / quoted mixed-case identity / equivalent reset and isolation -> tests/queries/access_control/test_alter_user.py::test_alter_user_self_target_query_tag[named-quoted] Named target / missing user / existing error -> tests/queries/access_control/test_alter_user.py::test_alter_user_nonexistent_error Named target / IF EXISTS missing user / SET and UNSET no-op -> tests/queries/access_control/test_alter_user.py::test_alter_user_if_exists_missing_comment Named target / IF EXISTS existing user / SET and UNSET state -> tests/queries/access_control/test_alter_user.py::test_alter_user_if_exists_existing_comment Named target / RENAME round-trip / unchanged existing behavior -> tests/queries/access_control/test_alter_user.py::test_alter_user_rename_round_trip Named target / property SET and UNSET / unchanged DESCRIBE readback -> tests/queries/access_control/test_desc_user.py::test_desc_user_after_alter_set_and_unset Parser / SET and UNSET / omitted name with and without IF EXISTS round-trips -> vendor/sqlparser/tests/sqlparser_common.rs::test_parse_alter_user Parser / action-keyword names / unquoted and quoted named SET and UNSET round-trips -> vendor/sqlparser/tests/sqlparser_common.rs::test_parse_alter_user Parser / action-keyword names / existing target-only statements round-trip -> vendor/sqlparser/tests/sqlparser_common.rs::test_parse_alter_user The resource-index ALTER USER syntax was enumerated. Independent action, authentication, MFA, authorization, policy, tag, and parameter semantics remain with their existing owners as specified; the SQL-visible target resolution proof uses only the already captured QUERY_TAG mutations. * LAV-4493: preserve the existing compat-test import Keep the existing is_real_snowflake import unchanged and add the disposable key-pair helper as a separate import. The branch's aggregate diff now only adds test dependencies and new tests rather than replacing an existing compat-test line. No waiver, assertion change, or snapshot migration is needed; the human-captured Cloud baselines remain unchanged. Validation: - make check: passed, including vendored sqlparser tests. - Focused ALTER USER and DESCRIBE USER compat replay: 45 passed. - Existing-test removal predicate against origin/main and the working tree: zero violations (the rejected import was the only violation). - git diff --check: passed. Co-authored-by: Przemek Denkiewicz <przemyslaw.denkiewicz@localstack.cloud> ## Test matrix Unnamed SET / unquoted identity / outcome and isolated state -> tests/queries/access_control/test_alter_user.py::test_alter_user_self_target_query_tag[unnamed-unquoted] Unnamed UNSET / unquoted identity / inherited default and isolated state -> tests/queries/access_control/test_alter_user.py::test_alter_user_self_target_query_tag[unnamed-unquoted] Unnamed SET / quoted identity / outcome and isolated state -> tests/queries/access_control/test_alter_user.py::test_alter_user_self_target_query_tag[unnamed-quoted] Unnamed UNSET / quoted identity / inherited default and isolated state -> tests/queries/access_control/test_alter_user.py::test_alter_user_self_target_query_tag[unnamed-quoted] Named SET / unquoted identity / equivalent outcome and isolated state -> tests/queries/access_control/test_alter_user.py::test_alter_user_self_target_query_tag[named-unquoted] Named UNSET / unquoted identity / equivalent inherited default -> tests/queries/access_control/test_alter_user.py::test_alter_user_self_target_query_tag[named-unquoted] Named SET / quoted identity / equivalent outcome and isolated state -> tests/queries/access_control/test_alter_user.py::test_alter_user_self_target_query_tag[named-quoted] Named UNSET / quoted identity / equivalent inherited default -> tests/queries/access_control/test_alter_user.py::test_alter_user_self_target_query_tag[named-quoted] Named target / missing user / error -> tests/queries/access_control/test_alter_user.py::test_alter_user_nonexistent_error Named target / IF EXISTS missing user / SET and UNSET no-op -> tests/queries/access_control/test_alter_user.py::test_alter_user_if_exists_missing_comment Named target / IF EXISTS existing user / SET and UNSET -> tests/queries/access_control/test_alter_user.py::test_alter_user_if_exists_existing_comment Named properties / SET and UNSET / DESCRIBE readback -> tests/queries/access_control/test_desc_user.py::test_desc_user_after_alter_set_and_unset Parser / omitted target and action-keyword named targets / round-trip -> vendor/sqlparser/tests/sqlparser_common.rs::test_parse_alter_user Re-enumerated the offline ALTER USER resource-index syntax. This feedback round changes imports only; independently owned actions and parameter semantics remain outside this target-resolution issue's contract. * LAV-4493: select ALTER USER tests when their readback helper changes Extend the existing test_desc_user.py dependency-selection rule to include its new test_alter_user.py importer. Preserve the existing user-actions selection and all test assertions, imports, and human-captured snapshots. Reproduced the feedback failure before the change with the existing transitive importer regression test. After the fix: - scripts/test_select_compat_tests.py: 228 passed. - make check: passed, including vendored sqlparser tests. - make test-compat PYTEST_PATH="tests/queries/access_control/test_alter_user.py tests/queries/access_control/test_desc_user.py" PYTEST_ARGS="-k 'alter_user or desc_user_after_alter_set_and_unset'": 26 passed. - git diff --check: passed. Co-authored-by: Przemek Denkiewicz <przemyslaw.denkiewicz@localstack.cloud> ## Test matrix Changed helper / direct and transitive importer selection -> scripts/test_select_compat_tests.py::test_changed_imported_test_module_selects_all_its_importers Unnamed SET / unquoted identity / outcome and isolated state -> tests/queries/access_control/test_alter_user.py::test_alter_user_self_target_query_tag[unnamed-unquoted] Unnamed UNSET / unquoted identity / inherited default and isolated state -> tests/queries/access_control/test_alter_user.py::test_alter_user_self_target_query_tag[unnamed-unquoted] Unnamed SET / quoted identity / outcome and isolated state -> tests/queries/access_control/test_alter_user.py::test_alter_user_self_target_query_tag[unnamed-quoted] Unnamed UNSET / quoted identity / inherited default and isolated state -> tests/queries/access_control/test_alter_user.py::test_alter_user_self_target_query_tag[unnamed-quoted] Named SET / unquoted identity / equivalent outcome and isolated state -> tests/queries/access_control/test_alter_user.py::test_alter_user_self_target_query_tag[named-unquoted] Named UNSET / unquoted identity / equivalent inherited default -> tests/queries/access_control/test_alter_user.py::test_alter_user_self_target_query_tag[named-unquoted] Named SET / quoted identity / equivalent outcome and isolated state -> tests/queries/access_control/test_alter_user.py::test_alter_user_self_target_query_tag[named-quoted] Named UNSET / quoted identity / equivalent inherited default -> tests/queries/access_control/test_alter_user.py::test_alter_user_self_target_query_tag[named-quoted] Named target / missing user / error -> tests/queries/access_control/test_alter_user.py::test_alter_user_nonexistent_error Named target / IF EXISTS missing user / SET and UNSET no-op -> tests/queries/access_control/test_alter_user.py::test_alter_user_if_exists_missing_comment Named target / IF EXISTS existing user / SET and UNSET -> tests/queries/access_control/test_alter_user.py::test_alter_user_if_exists_existing_comment Named properties / SET and UNSET / DESCRIBE readback -> tests/queries/access_control/test_desc_user.py::test_desc_user_after_alter_set_and_unset Parser / omitted target and action-keyword named targets / round-trip -> vendor/sqlparser/tests/sqlparser_common.rs::test_parse_alter_user Re-enumerated the offline ALTER USER resource-index syntax. This round changes only CI test selection; independent action and parameter semantics remain outside the target-resolution contract. No Cloud recapture was performed. --------- Co-authored-by: spiral <spiral@localhost> Co-authored-by: Przemek Denkiewicz <przemyslaw.denkiewicz@localstack.cloud> Co-authored-by: Przemek Denkiewicz <przemyslaw.denkiewicz@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 9714238 commit 722c64e

3 files changed

Lines changed: 55 additions & 5 deletions

File tree

‎src/ast/mod.rs‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17772,8 +17772,8 @@ impl fmt::Display for CreateWarehouse {
1777217772
pub struct AlterUser {
1777317773
/// Whether to only alter the user if it exists.
1777417774
pub if_exists: bool,
17775-
/// The name of the user to alter.
17776-
pub name: Ident,
17775+
/// The name of the user to alter, or the current user when omitted.
17776+
pub name: Option<Ident>,
1777717777
/// Optional new name for the user (Snowflake-specific).
1777817778
/// See: <https://docs.snowflake.com/en/sql-reference/sql/alter-user#syntax>
1777917779
pub rename_to: Option<Ident>,
@@ -17929,7 +17929,9 @@ impl fmt::Display for AlterUser {
1792917929
if self.if_exists {
1793017930
write!(f, " IF EXISTS")?;
1793117931
}
17932-
write!(f, " {}", self.name)?;
17932+
if let Some(name) = &self.name {
17933+
write!(f, " {name}")?;
17934+
}
1793317935
if let Some(new_name) = &self.rename_to {
1793417936
write!(f, " RENAME TO {new_name}")?;
1793517937
}

‎src/parser/alter.rs‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,33 @@ impl Parser<'_> {
150150
/// ```
151151
pub fn parse_alter_user(&mut self) -> Result<AlterUser, ParserError> {
152152
let if_exists = self.parse_keywords(&[Keyword::IF, Keyword::EXISTS]);
153-
let name = self.parse_identifier()?;
153+
let action_keywords = [
154+
Keyword::RENAME,
155+
Keyword::RESET,
156+
Keyword::ABORT,
157+
Keyword::ADD,
158+
Keyword::REMOVE,
159+
Keyword::ENROLL,
160+
Keyword::MODIFY,
161+
Keyword::SET,
162+
Keyword::UNSET,
163+
];
164+
let next_is_action = matches!(
165+
&self.peek_nth_token_ref(1).token,
166+
Token::Word(word)
167+
if action_keywords.contains(&word.keyword) || word.keyword == Keyword::WITH
168+
);
169+
let at_end = matches!(self.peek_nth_token_ref(1).token, Token::EOF | Token::SemiColon);
170+
let name = if action_keywords
171+
.iter()
172+
.any(|keyword| self.peek_keyword(*keyword))
173+
&& !next_is_action
174+
&& !at_end
175+
{
176+
None
177+
} else {
178+
Some(self.parse_identifier()?)
179+
};
154180
let _ = self.parse_keyword(Keyword::WITH);
155181
let rename_to = if self.parse_keywords(&[Keyword::RENAME, Keyword::TO]) {
156182
Some(self.parse_identifier()?)

‎tests/sqlparser_common.rs‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19372,13 +19372,35 @@ fn parse_create_index_different_using_positions() {
1937219372
#[test]
1937319373
fn test_parse_alter_user() {
1937419374
let dialects = all_dialects_except(|d| d.supports_alter_user_as_alter_role());
19375+
for action in ["SET QUERY_TAG='self-target-tag'", "UNSET QUERY_TAG"] {
19376+
for modifier in ["", "IF EXISTS "] {
19377+
let statement = dialects.verified_stmt(&format!("ALTER USER {modifier}{action}"));
19378+
let Statement::AlterUser(alter_user) = statement else {
19379+
unreachable!()
19380+
};
19381+
assert_eq!(alter_user.name, None);
19382+
assert_eq!(alter_user.if_exists, !modifier.is_empty());
19383+
}
19384+
for name in [
19385+
"SET", "UNSET", "RENAME", "RESET", "ABORT", "ADD", "REMOVE", "ENROLL", "MODIFY",
19386+
"WITH", "PASSWORD", "ENCRYPTED",
19387+
] {
19388+
dialects.verified_stmt(&format!("ALTER USER {name}"));
19389+
let statement = dialects.verified_stmt(&format!("ALTER USER {name} {action}"));
19390+
let Statement::AlterUser(alter_user) = statement else {
19391+
unreachable!()
19392+
};
19393+
assert_eq!(alter_user.name, Some(Ident::new(name)));
19394+
dialects.verified_stmt(&format!("ALTER USER \"{name}\" {action}"));
19395+
}
19396+
}
1937519397
dialects.verified_stmt("ALTER USER u1");
1937619398
dialects.verified_stmt("ALTER USER IF EXISTS u1");
1937719399
let stmt = dialects.verified_stmt("ALTER USER IF EXISTS u1 RENAME TO u2");
1937819400
match stmt {
1937919401
Statement::AlterUser(alter) => {
1938019402
assert!(alter.if_exists);
19381-
assert_eq!(alter.name, Ident::new("u1"));
19403+
assert_eq!(alter.name, Some(Ident::new("u1")));
1938219404
assert_eq!(alter.rename_to, Some(Ident::new("u2")));
1938319405
}
1938419406
_ => unreachable!(),

0 commit comments

Comments
 (0)