From 1b021f24f0d2aabacb10d53fe03cc175b99dde06 Mon Sep 17 00:00:00 2001 From: rakeshkky <12475069+rakeshkky@users.noreply.github.com> Date: Tue, 4 Aug 2020 17:26:10 +0530 Subject: [PATCH 1/6] restrict env variables start with HASURA_GRAPHQL_ for headers definition in actions & event triggers --- CHANGELOG.md | 3 ++- server/src-lib/Hasura/RQL/DDL/Headers.hs | 7 +++++-- .../actions/metadata/create_with_headers.yaml | 21 +++++++++++++++++++ server/tests-py/test_actions.py | 3 +++ 4 files changed, 31 insertions(+), 3 deletions(-) create mode 100644 server/tests-py/queries/actions/metadata/create_with_headers.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index 96884e474b27c..8d5cddcedd890 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ (Add entries here in the order of: server, console, cli, docs, others) +- server: restrict env variables start with HASURA_GRAPHQL_ for headers definition in actions & event triggers - server: bugfix to allow HASURA_GRAPHQL_QUERY_PLAN_CACHE_SIZE of 0 (#5363) - server: support only a bounded plan cache, with a default size of 4000 (closes #5363) - server: add logs for action handlers @@ -38,7 +39,7 @@ - server: have haskell runtime release blocks of memory back to the OS eagerly (related to #3388) - server: unlock locked scheduled events on graceful shutdown (#4928) - server: disable prepared statements for mutations as we end up with single-use objects which result in excessive memory consumption for mutation heavy workloads (#5255) -- server: include scheduled event metadata (`created_at`,`scheduled_time`,`id`, etc) along with the configured payload in the request body to the webhook. +- server: include scheduled event metadata (`created_at`,`scheduled_time`,`id`, etc) along with the configured payload in the request body to the webhook. **WARNING:** This is breaking for beta versions as the payload is now inside a key called `payload`. - console: allow configuring statement timeout on console RawSQL page (close #4998) (#5045) - console: support tracking partitioned tables (close #5071) (#5258) diff --git a/server/src-lib/Hasura/RQL/DDL/Headers.hs b/server/src-lib/Hasura/RQL/DDL/Headers.hs index 4bbf89a19d66a..87a3ed9d4f9bb 100644 --- a/server/src-lib/Hasura/RQL/DDL/Headers.hs +++ b/server/src-lib/Hasura/RQL/DDL/Headers.hs @@ -8,8 +8,8 @@ import Hasura.RQL.Types.Error import Language.Haskell.TH.Syntax (Lift) import qualified Data.CaseInsensitive as CI -import qualified Data.Text as T import qualified Data.Environment as Env +import qualified Data.Text as T import qualified Network.HTTP.Types as HTTP @@ -35,7 +35,10 @@ instance FromJSON HeaderConf where case (value, valueFromEnv ) of (Nothing, Nothing) -> fail "expecting value or value_from_env keys" (Just val, Nothing) -> return $ HeaderConf name (HVValue val) - (Nothing, Just val) -> return $ HeaderConf name (HVEnv val) + (Nothing, Just val) -> do + when (T.isPrefixOf "HASURA_GRAPHQL_" val) $ + fail "env variables start with \"HASURA_GRAPHQL_\" are not allowed for value_from_env" + return $ HeaderConf name (HVEnv val) (Just _, Just _) -> fail "expecting only one of value or value_from_env keys" parseJSON _ = fail "expecting object for headers" diff --git a/server/tests-py/queries/actions/metadata/create_with_headers.yaml b/server/tests-py/queries/actions/metadata/create_with_headers.yaml new file mode 100644 index 0000000000000..4e20e9e17f453 --- /dev/null +++ b/server/tests-py/queries/actions/metadata/create_with_headers.yaml @@ -0,0 +1,21 @@ +description: Define an action with headers configuration +url: /v1/query +status: 400 +query: + type: create_action + args: + name: create_user_1 + definition: + kind: synchronous + arguments: + - name: name + type: String! + output_type: User! + handler: http://127.0.0.1:5593/create-user + headers: + - name: x-client-id + value_from_env: HASURA_GRAPHQL_CLIENT_NAME +response: + path: $.definition.headers[0] + error: env variables start with "HASURA_GRAPHQL_" are not allowed for value_from_env + code: parse-failed diff --git a/server/tests-py/test_actions.py b/server/tests-py/test_actions.py index b4a7c5a57c1f6..849a532bcb15f 100644 --- a/server/tests-py/test_actions.py +++ b/server/tests-py/test_actions.py @@ -455,6 +455,9 @@ def dir(cls): def test_recreate_permission(self, hge_ctx): check_query_f(hge_ctx, self.dir() + '/recreate_permission.yaml') + def test_create_with_headers(self, hge_ctx): + check_query_f(hge_ctx, self.dir() + '/create_with_headers.yaml') + # Test case for bug reported at https://github.com/hasura/graphql-engine/issues/5166 @pytest.mark.usefixtures('per_class_tests_db_state') class TestActionIntrospection: From 74c6b8e27c257c778c92d2ee218919c201a72ca3 Mon Sep 17 00:00:00 2001 From: rakeshkky <12475069+rakeshkky@users.noreply.github.com> Date: Tue, 4 Aug 2020 19:58:29 +0530 Subject: [PATCH 2/6] update CHANGELOG.md --- CHANGELOG.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d5cddcedd890..c1f1ef72084dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,11 +2,20 @@ ## Next release +### Breaking change + +Environment variables starting with `HASURA_GRAPHQL_` are restricted to configure `value_from_env` key +in headers definition of event triggers, actions & remote schemas. The GraphQL Engine isn't upgradable +to this version if the metadata contains any action/event trigger/remote schema whose headers are defined +with restricted environment variable. + +To upgrade, recreate or update the action/event trigger/remote without restricted environment variable. + ### Bug fixes and improvements (Add entries here in the order of: server, console, cli, docs, others) -- server: restrict env variables start with HASURA_GRAPHQL_ for headers definition in actions & event triggers +- server: restrict env variables start with HASURA_GRAPHQL_ for headers definition in actions, event triggers & remote schemas (#5519) - server: bugfix to allow HASURA_GRAPHQL_QUERY_PLAN_CACHE_SIZE of 0 (#5363) - server: support only a bounded plan cache, with a default size of 4000 (closes #5363) - server: add logs for action handlers From ff2b754dd0c077070801921ab8de4047b995cd25 Mon Sep 17 00:00:00 2001 From: Rakesh Emmadi <12475069+rakeshkky@users.noreply.github.com> Date: Wed, 5 Aug 2020 14:04:18 +0530 Subject: [PATCH 3/6] Apply suggestions from code review Co-authored-by: Vamshi Surabhi <0x777@users.noreply.github.com> --- server/src-lib/Hasura/RQL/DDL/Headers.hs | 2 +- .../tests-py/queries/actions/metadata/create_with_headers.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src-lib/Hasura/RQL/DDL/Headers.hs b/server/src-lib/Hasura/RQL/DDL/Headers.hs index 87a3ed9d4f9bb..c2231c49678b2 100644 --- a/server/src-lib/Hasura/RQL/DDL/Headers.hs +++ b/server/src-lib/Hasura/RQL/DDL/Headers.hs @@ -37,7 +37,7 @@ instance FromJSON HeaderConf where (Just val, Nothing) -> return $ HeaderConf name (HVValue val) (Nothing, Just val) -> do when (T.isPrefixOf "HASURA_GRAPHQL_" val) $ - fail "env variables start with \"HASURA_GRAPHQL_\" are not allowed for value_from_env" + fail $ "env variables starting with \"HASURA_GRAPHQL_\" are not allowed in value_from_env: " <> T.unpack val return $ HeaderConf name (HVEnv val) (Just _, Just _) -> fail "expecting only one of value or value_from_env keys" parseJSON _ = fail "expecting object for headers" diff --git a/server/tests-py/queries/actions/metadata/create_with_headers.yaml b/server/tests-py/queries/actions/metadata/create_with_headers.yaml index 4e20e9e17f453..96d42b180eedb 100644 --- a/server/tests-py/queries/actions/metadata/create_with_headers.yaml +++ b/server/tests-py/queries/actions/metadata/create_with_headers.yaml @@ -17,5 +17,5 @@ query: value_from_env: HASURA_GRAPHQL_CLIENT_NAME response: path: $.definition.headers[0] - error: env variables start with "HASURA_GRAPHQL_" are not allowed for value_from_env + error: env variables starting with "HASURA_GRAPHQL_" are not allowed in value_from_env: HASURA_GRAPHQL_CLIENT_NAME code: parse-failed From 830c422191f4c9e940cab98fba7e25cad9278753 Mon Sep 17 00:00:00 2001 From: Tirumarai Selvan Date: Wed, 5 Aug 2020 15:00:17 +0530 Subject: [PATCH 4/6] Update CHANGELOG.md --- CHANGELOG.md | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c1f1ef72084dc..f437971e9ce4b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,18 +4,17 @@ ### Breaking change -Environment variables starting with `HASURA_GRAPHQL_` are restricted to configure `value_from_env` key -in headers definition of event triggers, actions & remote schemas. The GraphQL Engine isn't upgradable -to this version if the metadata contains any action/event trigger/remote schema whose headers are defined -with restricted environment variable. +Environment variables starting with `HASURA_GRAPHQL_` are not allowed to be set in header configuration +of event triggers, actions & remote schemas. -To upgrade, recreate or update the action/event trigger/remote without restricted environment variable. +If you do have such headers configured, then you must update the header configuration before upgrading. ### Bug fixes and improvements (Add entries here in the order of: server, console, cli, docs, others) -- server: restrict env variables start with HASURA_GRAPHQL_ for headers definition in actions, event triggers & remote schemas (#5519) +- server: disallow env variables starting with `HASURA_GRAPHQL_` for headers in actions, event triggers & remote schemas (#5519) +**WARNING**: This might break certain deployments. See `Breaking change` section above. - server: bugfix to allow HASURA_GRAPHQL_QUERY_PLAN_CACHE_SIZE of 0 (#5363) - server: support only a bounded plan cache, with a default size of 4000 (closes #5363) - server: add logs for action handlers From 97facd65573adb43905a8033db4eb5379c556864 Mon Sep 17 00:00:00 2001 From: rakeshkky <12475069+rakeshkky@users.noreply.github.com> Date: Wed, 5 Aug 2020 15:18:24 +0530 Subject: [PATCH 5/6] fix test yaml file --- .../tests-py/queries/actions/metadata/create_with_headers.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/tests-py/queries/actions/metadata/create_with_headers.yaml b/server/tests-py/queries/actions/metadata/create_with_headers.yaml index 96d42b180eedb..0a10a459546df 100644 --- a/server/tests-py/queries/actions/metadata/create_with_headers.yaml +++ b/server/tests-py/queries/actions/metadata/create_with_headers.yaml @@ -17,5 +17,5 @@ query: value_from_env: HASURA_GRAPHQL_CLIENT_NAME response: path: $.definition.headers[0] - error: env variables starting with "HASURA_GRAPHQL_" are not allowed in value_from_env: HASURA_GRAPHQL_CLIENT_NAME + error: 'env variables starting with "HASURA_GRAPHQL_" are not allowed in value_from_env: HASURA_GRAPHQL_CLIENT_NAME' code: parse-failed From 0bf1223539a777cb16f41a85d249d46be38d5500 Mon Sep 17 00:00:00 2001 From: Tirumarai Selvan Date: Wed, 5 Aug 2020 15:59:26 +0530 Subject: [PATCH 6/6] Update CHANGELOG.md --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f437971e9ce4b..47ce98b1e9e89 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,8 @@ ### Breaking change -Environment variables starting with `HASURA_GRAPHQL_` are not allowed to be set in header configuration -of event triggers, actions & remote schemas. +Headers from environment variables starting with `HASURA_GRAPHQL_` are not allowed +in event triggers, actions & remote schemas. If you do have such headers configured, then you must update the header configuration before upgrading. @@ -13,7 +13,7 @@ If you do have such headers configured, then you must update the header configur (Add entries here in the order of: server, console, cli, docs, others) -- server: disallow env variables starting with `HASURA_GRAPHQL_` for headers in actions, event triggers & remote schemas (#5519) +- server: disallow headers from env variables starting with `HASURA_GRAPHQL_` in actions, event triggers & remote schemas (#5519) **WARNING**: This might break certain deployments. See `Breaking change` section above. - server: bugfix to allow HASURA_GRAPHQL_QUERY_PLAN_CACHE_SIZE of 0 (#5363) - server: support only a bounded plan cache, with a default size of 4000 (closes #5363)