postgresql_tablespace: add trust_input parameter (#240)

* postgresql_tablespace: add trust_input parameter

* add changelog fragment

changelogs/fragments/240-postgresql_tablespace_add_trust_input_parameter.yml

@@ -0,0 +1,2 @@
+minor_changes:
+- postgresql_tablespace - add the ``trust_input`` parameter (https://github.com/ansible-collections/community.general/pull/240).

plugins/modules/database/postgresql/postgresql_tablespace.py

@@ -75,6 +75,11 @@ options:
     type: str
     aliases:
     - login_db
+  trust_input:
+    description:
+    - If C(no), check whether values of some parameters are potentially dangerous.
+    type: bool
+    default: yes
 
 notes:
 - I(state=absent) and I(state=present) (the second one if the tablespace doesn't exist) do not
@@ -185,7 +190,12 @@ except ImportError:
     pass
 
 from ansible.module_utils.basic import AnsibleModule
-from ansible_collections.community.general.plugins.module_utils.database import pg_quote_identifier
+from ansible.module_utils.six import iteritems
+
+from ansible_collections.community.general.plugins.module_utils.database import (
+    check_input,
+    pg_quote_identifier,
+)
 from ansible_collections.community.general.plugins.module_utils.postgres import (
     connect_to_db,
     exec_sql,
@@ -286,7 +296,7 @@ class PgTablespace(object):
         args:
             location (str) -- tablespace directory path in the FS
         """
-        query = ("CREATE TABLESPACE %s LOCATION '%s'" % (pg_quote_identifier(self.name, 'database'), location))
+        query = ('CREATE TABLESPACE "%s" LOCATION \'%s\'' % (self.name, location))
         return exec_sql(self, query, return_bool=True)
 
     def drop(self):
@@ -294,7 +304,7 @@ class PgTablespace(object):
 
         Return True if success, otherwise, return False.
         """
-        return exec_sql(self, "DROP TABLESPACE %s" % pg_quote_identifier(self.name, 'database'), return_bool=True)
+        return exec_sql(self, 'DROP TABLESPACE "%s"' % self.name, return_bool=True)
 
     def set_owner(self, new_owner):
         """Set tablespace owner.
@@ -307,7 +317,7 @@ class PgTablespace(object):
         if new_owner == self.owner:
             return False
 
-        query = "ALTER TABLESPACE %s OWNER TO %s" % (pg_quote_identifier(self.name, 'database'), new_owner)
+        query = 'ALTER TABLESPACE "%s" OWNER TO "%s"' % (self.name, new_owner)
         return exec_sql(self, query, return_bool=True)
 
     def rename(self, newname):
@@ -318,7 +328,7 @@ class PgTablespace(object):
         args:
             newname (str) -- new name for the tablespace"
         """
-        query = "ALTER TABLESPACE %s RENAME TO %s" % (pg_quote_identifier(self.name, 'database'), newname)
+        query = 'ALTER TABLESPACE "%s" RENAME TO "%s"' % (self.name, newname)
         self.new_name = newname
         return exec_sql(self, query, return_bool=True)
 
@@ -357,7 +367,7 @@ class PgTablespace(object):
         args:
             setting (str) -- string in format "setting_name = 'setting_value'"
         """
-        query = "ALTER TABLESPACE %s RESET (%s)" % (pg_quote_identifier(self.name, 'database'), setting)
+        query = 'ALTER TABLESPACE "%s" RESET (%s)' % (self.name, setting)
         return exec_sql(self, query, return_bool=True)
 
     def __set_setting(self, setting):
@@ -368,7 +378,7 @@ class PgTablespace(object):
         args:
             setting (str) -- string in format "setting_name = 'setting_value'"
         """
-        query = "ALTER TABLESPACE %s SET (%s)" % (pg_quote_identifier(self.name, 'database'), setting)
+        query = 'ALTER TABLESPACE "%s" SET (%s)' % (self.name, setting)
         return exec_sql(self, query, return_bool=True)
 
 
@@ -388,6 +398,7 @@ def main():
         rename_to=dict(type='str'),
         db=dict(type='str', aliases=['login_db']),
         session_role=dict(type='str'),
+        trust_input=dict(type='bool', default=True),
     )
 
     module = AnsibleModule(
@@ -402,11 +413,23 @@ def main():
     owner = module.params["owner"]
     rename_to = module.params["rename_to"]
     settings = module.params["set"]
+    session_role = module.params["session_role"]
+    trust_input = module.params["trust_input"]
 
     if state == 'absent' and (location or owner or rename_to or settings):
         module.fail_json(msg="state=absent is mutually exclusive location, "
                              "owner, rename_to, and set")
 
+    if not trust_input:
+        # Check input for potentially dangerous elements:
+        if not settings:
+            settings_list = None
+        else:
+            settings_list = ['%s = %s' % (k, v) for k, v in iteritems(settings)]
+
+        check_input(module, tablespace, location, owner,
+                    rename_to, session_role, settings_list)
+
     conn_params = get_conn_params(module, module.params, warn_db_default=False)
     db_connection = connect_to_db(module, conn_params, autocommit=True)
     cursor = db_connection.cursor(cursor_factory=DictCursor)
@@ -428,7 +451,8 @@ def main():
 
     # If tablespace exists with different location, exit:
     if tblspace.exists and location and location != tblspace.location:
-        module.fail_json(msg="Tablespace '%s' exists with different location '%s'" % (tblspace.name, tblspace.location))
+        module.fail_json(msg="Tablespace '%s' exists with "
+                             "different location '%s'" % (tblspace.name, tblspace.location))
 
     # Create new tablespace:
     if not tblspace.exists and state == 'present':

tests/integration/targets/postgresql_tablespace/defaults/main.yml

@@ -1,2 +1,3 @@
 ---
 test_tablespace_path: "/ssd"
+dangerous_name: 'curious.anonymous"; SELECT * FROM information_schema.tables; --'

tests/integration/targets/postgresql_tablespace/tasks/postgresql_tablespace_initial.yml

@@ -4,10 +4,12 @@
     path: '{{ test_tablespace_path }}'
     state: absent
   ignore_errors: true
+
 - name: postgresql_tablespace - disable selinux
   become: true
   shell: setenforce 0
   ignore_errors: true
+
 - name: postgresql_tablespace - create dir for test tablespace
   become: true
   file:
@@ -17,6 +19,7 @@
     group: '{{ pg_user }}'
     mode: '0700'
   ignore_errors: true
+
 - name: postgresql_tablespace - create test role to test change ownership
   become_user: '{{ pg_user }}'
   become: true
@@ -26,6 +29,7 @@
     name: bob
     state: present
   ignore_errors: true
+
 - name: postgresql_tablespace - create test role to test change ownership
   become_user: '{{ pg_user }}'
   become: true
@@ -35,6 +39,7 @@
     name: alice
     state: present
   ignore_errors: true
+
 - name: postgresql_tablespace - create a new tablespace called acme and set bob as an its owner
   become_user: '{{ pg_user }}'
   become: true
@@ -46,15 +51,17 @@
     location: /ssd
   register: result
   ignore_errors: true
+
 - assert:
     that:
     - result is changed
     - result.owner == 'bob'
-    - result.queries == ["CREATE TABLESPACE \"acme\" LOCATION '/ssd'", "ALTER TABLESPACE \"acme\" OWNER TO bob"]
+    - result.queries == ["CREATE TABLESPACE \"acme\" LOCATION '/ssd'", "ALTER TABLESPACE \"acme\" OWNER TO \"bob\""]
     - result.state == 'present'
     - result.tablespace == 'acme'
     - result.options == {}
     - result.location == '/ssd'
+
 - name: postgresql_tablespace - try to create the same tablespace with different location
   become_user: '{{ pg_user }}'
   become: true
@@ -65,10 +72,12 @@
     location: /another-ssd
   register: result
   ignore_errors: true
+
 - assert:
     that:
     - result is not changed
     - result.msg == "Tablespace 'acme' exists with different location '/ssd'"
+
 - name: postgresql_tablespace - change tablespace owner to alice
   become_user: '{{ pg_user }}'
   become: true
@@ -79,14 +88,16 @@
     owner: alice
   register: result
   ignore_errors: true
+
 - assert:
     that:
     - result is changed
     - result.owner == 'alice'
-    - result.queries == ["ALTER TABLESPACE \"acme\" OWNER TO alice"]
+    - result.queries == ["ALTER TABLESPACE \"acme\" OWNER TO \"alice\""]
     - result.state == 'present'
     - result.tablespace == 'acme'
     - result.options == {}
+
 - name: postgresql_tablespace - try to change tablespace owner to alice again to be sure that nothing changes
   become_user: '{{ pg_user }}'
   become: true
@@ -97,6 +108,7 @@
     owner: alice
   register: result
   ignore_errors: true
+
 - assert:
     that:
     - result is not changed
@@ -105,6 +117,7 @@
     - result.state == 'present'
     - result.tablespace == 'acme'
     - result.options == {}
+
 - name: postgresql_tablespace - change tablespace options
   become_user: '{{ pg_user }}'
   become: true
@@ -116,6 +129,7 @@
       seq_page_cost: 4
   register: result
   ignore_errors: true
+
 - assert:
     that:
     - result is changed
@@ -125,6 +139,7 @@
     - result.tablespace == 'acme'
     - result.options.seq_page_cost == '4'
   when: postgres_version_resp.stdout is version('9.0', '>=')
+
 - name: postgresql_tablespace - reset seq_page_cost option
   become_user: '{{ pg_user }}'
   become: true
@@ -136,11 +151,13 @@
       seq_page_cost: reset
   register: result
   ignore_errors: true
+
 - assert:
     that:
     - result is changed
     - result.queries == ["ALTER TABLESPACE \"acme\" RESET (seq_page_cost)"]
   when: postgres_version_resp.stdout is version('9.0', '>=')
+
 - name: postgresql_tablespace - reset seq_page_cost option again
   become_user: '{{ pg_user }}'
   become: true
@@ -152,11 +169,13 @@
       seq_page_cost: reset
   register: result
   ignore_errors: true
+
 - assert:
     that:
     - result is not changed
     - result.queries == []
   when: postgres_version_resp.stdout is version('9.0', '>=')
+
 - name: postgresql_tablespace - rename tablespace
   become_user: '{{ pg_user }}'
   become: true
@@ -167,11 +186,30 @@
     rename_to: foo
   register: result
   ignore_errors: true
+
 - assert:
     that:
     - result is changed
     - result.newname == 'foo'
-    - result.queries == ["ALTER TABLESPACE \"acme\" RENAME TO foo"]
+    - result.queries == ["ALTER TABLESPACE \"acme\" RENAME TO \"foo\""]
+
+- name: postgresql_tablespace - rename tablespace to potentially dangerous name
+  become_user: '{{ pg_user }}'
+  become: true
+  postgresql_tablespace:
+    db: postgres
+    login_user: '{{ pg_user }}'
+    name: foo
+    rename_to: '{{ dangerous_name }}'
+    trust_input: no
+  register: result
+  ignore_errors: true
+
+- assert:
+    that:
+    - result is failed
+    - result.msg == 'Passed input \'{{ dangerous_name }}\' is potentially dangerous'
+
 - name: postgresql_tablespace - drop tablespace
   become_user: '{{ pg_user }}'
   become: true
@@ -180,13 +218,16 @@
     login_user: '{{ pg_user }}'
     name: foo
     state: absent
+    trust_input: yes
   register: result
   ignore_errors: true
+
 - assert:
     that:
     - result is changed
     - result.state == 'absent'
     - result.queries == ["DROP TABLESPACE \"foo\""]
+
 - name: postgresql_tablespace - try to drop nonexistent tablespace
   become_user: '{{ pg_user }}'
   become: true
@@ -197,6 +238,7 @@
     state: absent
   register: result
   ignore_errors: true
+
 - assert:
     that:
     - result is not changed