Repository navigation
fix(editor): tier etcd statements by the operation the driver parses so lease revoke is destructive - #3188
Merged
Conversation
…so lease revoke is destructive
Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
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.
Summary
lease revoke(which deletes every key attached to the lease),user delete,role deleteandauth disableskipped the destructive confirmation and were allowed for an external client that may write but may not run destructive statements. The classifier tiered etcd text by its first word against a hand-written verb list that does not match the commands the plugin's parser takes, so everylease,user,roleandauthsubcommand fell to the write tier; it now tiers the operationEtcdCommandParserreturns, in one exhaustive switch.member list,lease list,lease timetolive,user listandrole listwere gated as writes, so an external client with read-only access was refused and write confirmation prompted for them. Same root cause: the verb list never looked at the subcommand."del" /keys --prefixor"lease" revoke 7bdropped to a write. The first word was read raw instead of the way the driver's tokenizer reads it; text the parser rejects is now tiered by its leading word as the driver tokenizes it, failing closed to destructive for commands that can delete keys or remove access.Tests
In
TableProTests/Core/Utilities/SQL/QueryClassifierHardeningTests.swift(suiteQueryClassifierNonSqlTests):etcdDestructiveSubcommands:lease revokeand its upper-case and quoted spellings,user delete,role delete,auth disable, quoted"del",deleteandcompactionare destructive. Fails without the fix (7 expectations).etcdReadSubcommands:member list,lease list,lease timetolive,endpoint status,endpoint health,user list,role listand quoted'get'are safe. Fails without the fix (6 expectations).etcdUnparsedFailsClosed: rejected text whose command can destroy (del --from-key /a,lease revokewith no id,member remove) is destructive, other rejected text is a write,defragreaches the filesystem. Fails without the fix (3 expectations).etcdWriteSubcommands: grants, keep-alives, account additions andputstay writes. Passes before and after; it guards against over-tightening.The existing
etcdTierscase, the etcd cases inMCPStatementGateRefusalTestsandMCPStatementGateConsentPolicyTests, and theEtcdCommandParsersuites pass unchanged.Docs
The docs rewrite branch covers the user-facing text for which etcd commands ask for confirmation and which count as reads.