Merge pull request #6105 from influxdata/fix/auto-execution

feat: preventing ddl and dml statements from autosubmiting

CHANGELOG.md

@@ -3,7 +3,8 @@
 ### Bug Fixes
 
 1. [#6103](https://github.com/influxdata/chronograf/pull/6103): Set active database for InfluxQL meta queries.
-1. [#6111](https://github.com/influxdata/chronograf/pull/6111): Fix loading Hosts page for large number of hosts.
+2. [#6105](https://github.com/influxdata/chronograf/pull/6105): Prevent dangerous InfluxQL statements from auto-execution.
+3. [#6111](https://github.com/influxdata/chronograf/pull/6111): Loading Hosts page for large number of hosts.
 
 ### Other
 

ui/.eslintrc

@@ -182,7 +182,7 @@
         "SwitchCase": 1
       }
     ],
-    "linebreak-style": [2, "unix"],
+    "linebreak-style": 0,
     "lines-around-comment": 0,
     "max-depth": 0,
     "max-len": 0,

ui/src/dashboards/components/InfluxQLEditor.tsx

@@ -8,24 +8,32 @@ import ReactCodeMirror from 'src/dashboards/components/ReactCodeMirror'
 import TemplateDrawer from 'src/shared/components/TemplateDrawer'
 import QueryStatus from 'src/shared/components/QueryStatus'
 import {ErrorHandling} from 'src/shared/decorators/errors'
-import {Dropdown, DropdownMode, ComponentStatus} from 'src/reusable_ui'
+import {
-import {Button, ComponentColor, ComponentSize} from 'src/reusable_ui'
+  Button,
+  ComponentColor,
+  ComponentSize,
+  ComponentStatus,
+  Dropdown,
+  DropdownMode,
+} from 'src/reusable_ui'
 
 // Utils
 import {getDeep} from 'src/utils/wrappers'
 import {makeCancelable} from 'src/utils/promises'
 
 // Constants
-import {MATCH_INCOMPLETE_TEMPLATES, applyMasks} from 'src/tempVars/constants'
+import {applyMasks, MATCH_INCOMPLETE_TEMPLATES} from 'src/tempVars/constants'
-import {METAQUERY_TEMPLATE_OPTIONS} from 'src/data_explorer/constants'
+import {
+  DropdownChildTypes,
+  METAQUERY_TEMPLATE_OPTIONS,
+  MetaQueryTemplateOption,
+} from 'src/data_explorer/constants'
 
 // Types
-import {Template, QueryConfig} from 'src/types'
+import {QueryConfig, Template} from 'src/types'
 import {WrappedCancelablePromise} from 'src/types/promises'
-import {
+import {isExcludedStatement} from 'src/utils/queryFilter'
-  MetaQueryTemplateOption,
+import {ErrorSkipped} from 'src/types/queries'
-  DropdownChildTypes,
-} from 'src/data_explorer/constants'
 
 interface TempVar {
   tempVar: string
@@ -41,11 +49,12 @@ interface State {
   filteredTemplates: Template[]
   isSubmitted: boolean
   configID: string
+  isExcluded: boolean
 }
 
 interface Props {
   query: string
-  onUpdate: (text: string) => Promise<void>
+  onUpdate: (text: string, isAutoSubmitted: boolean) => Promise<void>
   config: QueryConfig
   templates: Template[]
   onMetaQuerySelected: () => void
@@ -64,21 +73,31 @@ const TEMPLATE_VAR = /[:]\w+[:]/g
 class InfluxQLEditor extends Component<Props, State> {
   public static getDerivedStateFromProps(nextProps: Props, prevState: State) {
     const {isSubmitted, editedQueryText} = prevState
-
+    const {query, config, templates} = nextProps
-    const isQueryConfigChanged = nextProps.config.id !== prevState.configID
+    const isQueryConfigChanged = config.id !== prevState.configID
-    const isQueryTextChanged = editedQueryText.trim() !== nextProps.query.trim()
+    const isQueryTextChanged = editedQueryText.trim() !== query.trim()
-
+    // if query has been switched, set submitted state for excluded query based on the previous submitted way
-    if ((isSubmitted && isQueryTextChanged) || isQueryConfigChanged) {
+    let submitted: boolean
+    if (isQueryConfigChanged) {
+      submitted = isExcludedStatement(query)
+        ? config.status?.error !== ErrorSkipped
+        : true
+    } else {
+      submitted = isSubmitted
+    }
+    if ((submitted && isQueryTextChanged) || isQueryConfigChanged) {
       return {
         ...BLURRED_EDITOR_STATE,
         selectedTemplate: {
-          tempVar: getDeep<string>(nextProps.templates, FIRST_TEMP_VAR, ''),
+          tempVar: getDeep<string>(templates, FIRST_TEMP_VAR, ''),
         },
-        filteredTemplates: nextProps.templates,
+        filteredTemplates: templates,
-        templatingQueryText: nextProps.query,
+        templatingQueryText: query,
-        editedQueryText: nextProps.query,
+        editedQueryText: query,
-        configID: nextProps.config.id,
+        configID: config.id,
         focused: isQueryConfigChanged,
+        isSubmitted: submitted,
+        isExcluded: isExcludedStatement(query),
       }
     }
 
@@ -102,8 +121,9 @@ class InfluxQLEditor extends Component<Props, State> {
       filteredTemplates: props.templates,
       templatingQueryText: props.query,
       editedQueryText: props.query,
-      isSubmitted: true,
       configID: props.config.id,
+      isSubmitted: true,
+      isExcluded: isExcludedStatement(props.query),
     }
   }
 
@@ -126,6 +146,7 @@ class InfluxQLEditor extends Component<Props, State> {
       isShowingTemplateValues,
       focused,
       isSubmitted,
+      isExcluded,
     } = this.state
 
     return (
@@ -159,9 +180,17 @@ class InfluxQLEditor extends Component<Props, State> {
           <div className="varmoji-container">
             <div className="varmoji-front">
               <QueryStatus
-                status={config.status}
+                status={
+                  config.status?.error === ErrorSkipped
+                    ? config.status.submittedStatus
+                    : config.status
+                }
                 isShowingTemplateValues={isShowingTemplateValues}
-                isSubmitted={isSubmitted}
+                isSubmitted={
+                  (isSubmitted && !isExcluded) ||
+                  (isExcluded &&
+                    templatingQueryText === config.status?.submittedQuery)
+                }
               >
                 {this.queryStatusButtons}
               </QueryStatus>
@@ -200,7 +229,7 @@ class InfluxQLEditor extends Component<Props, State> {
 
   private handleBlurEditor = (): void => {
     this.setState({focused: false, isShowingTemplateValues: false})
-    this.handleUpdate()
+    this.handleUpdate(true)
   }
 
   private handleCloseDrawer = (): void => {
@@ -245,7 +274,7 @@ class InfluxQLEditor extends Component<Props, State> {
 
     const isTemplating = matched && !_.isEmpty(templates)
     if (isTemplating) {
-      // maintain cursor poition
+      // maintain cursor position
       const matchedVar = {tempVar: `${matched[0]}:`}
       const filteredTemplates = this.filterTemplates(matched[0])
       const selectedTemplate = this.selectMatchingTemplate(
@@ -262,8 +291,10 @@ class InfluxQLEditor extends Component<Props, State> {
         isSubmitted,
       })
     } else {
+      const isExcluded = isExcludedStatement(value)
       this.setState({
         isTemplating,
+        isExcluded,
         templatingQueryText: value,
         editedQueryText: value,
         isSubmitted,
@@ -271,22 +302,26 @@ class InfluxQLEditor extends Component<Props, State> {
     }
   }
 
-  private handleUpdate = async (): Promise<void> => {
+  private handleUpdate = async (isAutoSubmitted?: boolean): Promise<void> => {
-    const {onUpdate} = this.props
+    const {onUpdate, config} = this.props
-
+    const {editedQueryText, isSubmitted, isExcluded} = this.state
-    if (!this.isDisabled && !this.state.isSubmitted) {
+    if (
-      const {editedQueryText} = this.state
+      !this.isDisabled &&
+      (!isSubmitted || (isExcluded && config.status?.error === ErrorSkipped))
+    ) {
       this.cancelPendingUpdates()
-      const update = onUpdate(editedQueryText)
+      const update = onUpdate(editedQueryText, isAutoSubmitted)
       const cancelableUpdate = makeCancelable(update)
 
       this.pendingUpdates = [...this.pendingUpdates, cancelableUpdate]
 
       try {
         await cancelableUpdate.promise
-
         // prevent changing submitted status when edited while awaiting update
-        if (this.state.editedQueryText === editedQueryText) {
+        if (
+          this.state.editedQueryText === editedQueryText &&
+          (!isExcluded || !isAutoSubmitted)
+        ) {
           this.setState({isSubmitted: true})
         }
       } catch (error) {
@@ -443,7 +478,7 @@ class InfluxQLEditor extends Component<Props, State> {
           size={ComponentSize.ExtraSmall}
           color={ComponentColor.Primary}
           status={this.isDisabled && ComponentStatus.Disabled}
-          onClick={this.handleUpdate}
+          onClick={() => this.handleUpdate()}
           text="Submit Query"
         />
       </div>

ui/src/dashboards/utils/cellGetters.ts

@@ -189,6 +189,7 @@ export const getConfig = async (
     // return back the raw query
     queryConfig.rawText = query
   }
+
   return {
     ...queryConfig,
     originalQuery: query,

ui/src/shared/apis/query.ts

@@ -4,17 +4,9 @@ import {TEMP_VAR_INTERVAL, DEFAULT_DURATION_MS} from 'src/shared/constants'
 import replaceTemplates, {replaceInterval} from 'src/tempVars/utils/replace'
 import {proxy} from 'src/utils/queryUrlGenerator'
 
-import {Source, Template} from 'src/types'
+import {Query, Source, Template} from 'src/types'
 import {TimeSeriesResponse} from 'src/types/series'
-
+import {ErrorSkipped} from 'src/types/queries'
-// REVIEW: why is this different than the `Query` in src/types?
-interface Query {
-  text: string
-  id: string
-  database?: string
-  db?: string
-  rp?: string
-}
 
 interface QueryResult {
   value: TimeSeriesResponse | null
@@ -33,6 +25,18 @@ export function executeQueries(
     let counter = queries.length
 
     for (let i = 0; i < queries.length; i++) {
+      const q = queries[i]
+      if (
+        q.queryConfig.isExcluded &&
+        !q.queryConfig.status.isManuallySubmitted
+      ) {
+        results[i] = {value: null, error: ErrorSkipped}
+        counter -= 1
+        if (counter === 0) {
+          resolve(results)
+        }
+        continue
+      }
       executeQuery(source, queries[i], templates, uuid)
         .then(result => (results[i] = {value: result, error: null}))
         .catch(result => (results[i] = {value: null, error: result}))
@@ -58,9 +62,9 @@ export const executeQuery = async (
 
   const {data} = await proxy({
     source: source.links.proxy,
-    rp: query.rp,
+    rp: query.queryConfig.retentionPolicy,
     query: text,
-    db: query.db || query.database,
+    db: query.queryConfig.database,
     uuid,
   })
 

ui/src/shared/components/TimeMachine/InfluxQLQueryMaker.tsx

@@ -50,7 +50,7 @@ interface PassedProps {
   templates: Template[]
   onAddQuery: () => void
   onDeleteQuery: (index: number) => void
-  onEditRawText: (text: string) => Promise<void>
+  onEditRawText: (text: string, isAutoSubmitted: boolean) => Promise<void>
   onMetaQuerySelected: () => void
 }
 

ui/src/shared/components/TimeMachine/TimeMachine.tsx

@@ -54,6 +54,7 @@ import {
 import {SourceOption} from 'src/types/sources'
 import {Links, ScriptStatus} from 'src/types/flux'
 import queryBuilderFetcher from './fluxQueryBuilder/apis/queryBuilderFetcher'
+import {isExcludedStatement} from 'src/utils/queryFilter'
 
 interface ConnectedProps {
   script: string
@@ -420,10 +421,20 @@ class TimeMachine extends PureComponent<Props, State> {
     return getDeep(queryDrafts, '0.source', '') === ''
   }
 
-  private handleEditRawText = async (text: string): Promise<void> => {
+  private handleEditRawText = async (
-    const {templates, onUpdateQueryDrafts, queryDrafts, notify} = this.props
+    text: string,
+    isAutoSubmitted: boolean
+  ): Promise<void> => {
+    const {
+      templates,
+      onUpdateQueryDrafts,
+      queryDrafts,
+      queryStatuses,
+      notify,
+    } = this.props
     const activeID = this.activeQuery.id
     const url: string = _.get(this.source, 'links.queries', '')
+    const isExcluded = isExcludedStatement(text)
 
     let newQueryConfig
 
@@ -446,11 +457,16 @@ class TimeMachine extends PureComponent<Props, State> {
         queryConfig: {
           ...newQueryConfig,
           rawText: text,
-          status: {loading: true},
+          isExcluded,
         },
       }
     })
-
+    this.handleEditQueryStatus(activeID, {
+      ...queryStatuses[activeID],
+      isManuallySubmitted: !isAutoSubmitted,
+      submittedStatus: queryStatuses[activeID]?.submittedStatus,
+      submittedQuery: queryStatuses[activeID]?.submittedQuery,
+    })
     onUpdateQueryDrafts(updatedQueryDrafts)
   }
 

ui/src/shared/components/time_series/TimeSeries.tsx

@@ -128,6 +128,20 @@ class TimeSeries extends PureComponent<Props, State> {
     const currQueries = _.map(this.props.queries, q => q.text)
     const queriesDifferent = !_.isEqual(prevQueries, currQueries)
 
+    let manualSubmit = false
+    if (!queriesDifferent) {
+      for (let i = 0; i < this.props.queries.length; i++) {
+        const query = this.props.queries[i]
+        const prevQuery = prevProps.queries[i]
+        if (
+          query.queryConfig?.status?.isManuallySubmitted &&
+          !prevQuery.queryConfig?.status?.isManuallySubmitted
+        ) {
+          manualSubmit = true
+          break
+        }
+      }
+    }
     const prevTemplates = _.get(prevProps, 'templates')
     const newTemplates = _.get(this.props, 'templates')
     // templates includes dashTime and upperDashTime which capture zoomedTimeRange
@@ -140,6 +154,7 @@ class TimeSeries extends PureComponent<Props, State> {
     const timeRangeChanged = oldLower !== newLower || oldUpper !== newUpper
 
     const shouldExecuteQueries =
+      manualSubmit ||
       queriesDifferent ||
       timeRangeChanged ||
       templatesDifferent ||
@@ -308,7 +323,13 @@ class TimeSeries extends PureComponent<Props, State> {
     const {source, templates, editQueryStatus, queries} = this.props
 
     for (const query of queries) {
-      editQueryStatus(query.id, {loading: true})
+      const prevStatus = query.queryConfig.status
+      editQueryStatus(query.id, {
+        loading: true,
+        isManuallySubmitted: prevStatus?.isManuallySubmitted,
+        submittedStatus: prevStatus?.submittedStatus,
+        submittedQuery: prevStatus?.submittedQuery,
+      })
     }
 
     const results = await this.executeInfluxQLQueries(
@@ -335,8 +356,18 @@ class TimeSeries extends PureComponent<Props, State> {
           queryStatus = {success: 'Success!'}
         }
       }
-
+      const shouldPreserve =
-      editQueryStatus(query.id, queryStatus)
+        query.queryConfig.isExcluded &&
+        !query.queryConfig.status?.isManuallySubmitted
+      editQueryStatus(query.id, {
+        ...queryStatus,
+        submittedStatus: shouldPreserve
+          ? query.queryConfig.status.submittedStatus
+          : queryStatus,
+        submittedQuery: shouldPreserve
+          ? query.queryConfig.status.submittedQuery
+          : query.text,
+      })
     }
 
     const validQueryResults = results

ui/src/shared/utils/TimeMachineContainer.ts

@@ -27,6 +27,7 @@ import {
   setLocalStorage,
   TMLocalStorageKey,
 } from 'src/shared/utils/timeMachine'
+import {isExcludedStatement} from 'src/utils/queryFilter'
 
 // Constants
 import {TYPE_QUERY_CONFIG} from 'src/dashboards/constants'
@@ -147,7 +148,7 @@ export class TimeMachineContainer {
       state = {...state, queryDrafts}
     }
 
-    // prevents "DROP" or "DELETE" queries from being persisted.
+    // prevents DDL and DML statements from being persisted.
     const savable = getDeep<CellQuery[]>(state, 'queryDrafts', []).filter(
       ({query, type}) => {
         if (type !== 'influxql') {
@@ -161,8 +162,8 @@ export class TimeMachineContainer {
         const queries = query.split(';')
         let isSavable = true
         for (let i = 0; i <= queries.length; i++) {
-          const qs = getDeep<string>(queries, `${i}`, '').toLocaleLowerCase()
+          const qs = getDeep<string>(queries, `${i}`, '')
-          if (qs.startsWith('drop') || qs.startsWith('delete')) {
+          if (isExcludedStatement(qs)) {
             isSavable = false
           }
         }

ui/src/types/queries.ts

@@ -1,6 +1,8 @@
 // Types
 import {Source} from 'src/types'
 
+export const ErrorSkipped = 'skipped'
+
 export interface Query {
   text: string
   id: string
@@ -27,6 +29,7 @@ export interface QueryConfig {
   upper?: string
   isQuerySupportedByExplorer?: boolean // doesn't come from server -- is set in CellEditorOverlay
   originalQuery?: string
+  isExcluded?: boolean
 }
 
 export interface QueryStatus {
@@ -86,9 +89,12 @@ export interface Namespace {
 
 export interface Status {
   loading?: boolean
+  isManuallySubmitted?: boolean
   error?: string
   warn?: string
   success?: string
+  submittedStatus?: Status
+  submittedQuery?: string
 }
 
 export interface TimeRange {

ui/src/utils/queryFilter.ts

@@ -0,0 +1,15 @@
+const excludedStatements: string[] = [
+  'drop',
+  'delete',
+  'alter',
+  'create',
+  'grant',
+  'revoke',
+  'use',
+]
+
+export const isExcludedStatement = (query: string): boolean => {
+  return excludedStatements.some(statement =>
+    query?.toLowerCase().startsWith(statement)
+  )
+}