web-cli quote parsing (#6755)

* upgrade yargs-parser for better quote handling * remove encoding pre&post parse, and remove wrapping quotes when pushing to data array * add test for spaces and strings * base64 encode policy strings in tests where we're using them with string interpolation * improve regex to only remove wrapping single and double quotes * don't support quotes in paths in the web cli
2026-05-05 12:26:34 +02:00 · 2019-05-22 16:07:42 -05:00 · 2019-05-22 16:07:42 -05:00 · eba703bec7
commit eba703bec7
parent 93796f371d
9 changed files with 116 additions and 86 deletions
--- a/ui/app/lib/console-helpers.js
+++ b/ui/app/lib/console-helpers.js
@ -56,14 +56,11 @@ export function executeUICommand(command, logAndOutput, clearLog, toggleFullscre
 }

 export function parseCommand(command, shouldThrow) {
-  // encode everything but spaces
-  let cmd = encodeURIComponent(command).replace(/%20/g, decodeURIComponent);
-  let args = argTokenizer(cmd);
+  let args = argTokenizer(command);
  if (args[0] === 'vault') {
    args.shift();
  }

-  args = args.map(decodeURIComponent);
  let [method, ...rest] = args;
  let path;
  let flags = [];
@ -74,7 +71,17 @@ export function parseCommand(command, shouldThrow) {
      flags.push(arg);
    } else {
      if (path) {
-        data.push(arg);
+        let strippedArg = arg
+          // we'll have arg=something or arg="lol I need spaces", so need to split on the first =
+          .split(/=(.+)/)
+          // remove matched wrapping " or ' from each item
+          .map(item => item.replace(/^("|')(.+)(\1)$/, '$2'))
+          // if there were quotes, there's an empty string as the last member in the array that we don't want,
+          // so filter it out
+          .filter(str => str !== '')
+          // glue the data back together
+          .join('=');
+        data.push(strippedArg);
      } else {
        path = arg;
      }
--- a/ui/app/utils/args-tokenizer.js
+++ b/ui/app/utils/args-tokenizer.js
@ -1,38 +0,0 @@
-export default function(argString) {
-  if (Array.isArray(argString)) return argString;
-
-  argString = argString.trim();
-
-  var i = 0;
-  var prevC = null;
-  var c = null;
-  var opening = null;
-  var args = [];
-
-  for (var ii = 0; ii < argString.length; ii++) {
-    prevC = c;
-    c = argString.charAt(ii);
-
-    // split on spaces unless we're in quotes.
-    if (c === ' ' && !opening) {
-      if (!(prevC === ' ')) {
-        i++;
-      }
-      continue;
-    }
-
-    // don't split the string if we're in matching
-    // opening or closing single and double quotes.
-    if (c === opening) {
-      if (!args[i]) args[i] = '';
-      opening = null;
-    } else if ((c === "'" || c === '"') && argString.indexOf(c, ii + 1) > 0 && !opening) {
-      opening = c;
-    }
-
-    if (!args[i]) args[i] = '';
-    args[i] += c;
-  }
-
-  return args;
-}
--- a/ui/package.json
+++ b/ui/package.json
@ -127,7 +127,7 @@
    "text-encoder-lite": "1.0.0",
    "walk-sync": "^0.3.3",
    "xstate": "^3.3.3",
-    "yargs-parser": "^10.0.0"
+    "yargs-parser": "^13.0.0"
  },
  "optionalDependencies": {
    "@babel/core": "^7.3.4",
--- a/ui/tests/acceptance/cluster-test.js
+++ b/ui/tests/acceptance/cluster-test.js
@ -9,7 +9,7 @@ const consoleComponent = create(consoleClass);

 const tokenWithPolicy = async function(name, policy) {
  await consoleComponent.runCommands([
-    `write sys/policies/acl/${name} policy=${policy}`,
+    `write sys/policies/acl/${name} policy=${btoa(policy)}`,
    `write -field=client_token auth/token/create policies=${name}`,
  ]);

@ -25,11 +25,11 @@ module('Acceptance | cluster', function(hooks) {
  });

  test('hides nav item if user does not have permission', async function(assert) {
-    const deny_policies_policy = `'
+    const deny_policies_policy = `
      path "sys/policies/*" {
        capabilities = ["deny"]
      },
-    '`;
+    `;

    const userToken = await tokenWithPolicy('hide-policies-nav', deny_policies_policy);
    await logout.visit();
@ -40,11 +40,11 @@ module('Acceptance | cluster', function(hooks) {
  });

  test('enterprise nav item links to first route that user has access to', async function(assert) {
-    const read_rgp_policy = `'
+    const read_rgp_policy = `
      path "sys/policies/rgp" {
        capabilities = ["read"]
      },
-    '`;
+    `;

    const userToken = await tokenWithPolicy('show-policies-nav', read_rgp_policy);
    await logout.visit();
--- a/ui/tests/acceptance/enterprise-control-groups-test.js
+++ b/ui/tests/acceptance/enterprise-control-groups-test.js
@ -27,7 +27,7 @@ module('Acceptance | Enterprise | control groups', function(hooks) {
    return logout.visit();
  });

-  const POLICY = `'
+  const POLICY = `
    path "kv/foo" {
      capabilities = ["create", "read", "update", "delete", "list"]
      control_group = {
@ -40,9 +40,9 @@ module('Acceptance | Enterprise | control groups', function(hooks) {
         }
      }
    }
-  '`;
+  `;

-  const AUTHORIZER_POLICY = `'
+  const AUTHORIZER_POLICY = `
    path "sys/control-group/authorize" {
      capabilities = ["update"]
    }
@ -50,7 +50,7 @@ module('Acceptance | Enterprise | control groups', function(hooks) {
    path "sys/control-group/request" {
      capabilities = ["update"]
    }
-  '`;
+  `;

  const ADMIN_USER = 'authorizer';
  const ADMIN_PASSWORD = 'test';
@ -67,8 +67,8 @@ module('Acceptance | Enterprise | control groups', function(hooks) {
      `write auth/userpass/users/${ADMIN_USER} password=${ADMIN_PASSWORD} policies=default`,
      `write identity/entity name=${ADMIN_USER} policies=test`,
      // write policies for control group + authorization
-      `write sys/policies/acl/kv-control-group policy=${POLICY}`,
-      `write sys/policies/acl/authorizer policy=${AUTHORIZER_POLICY}`,
+      `write sys/policies/acl/kv-control-group policy=${btoa(POLICY)}`,
+      `write sys/policies/acl/authorizer policy=${btoa(AUTHORIZER_POLICY)}`,
      // read out mount to get the accessor
      'read -field=accessor sys/internal/ui/mounts/auth/userpass',
    ]);
--- a/ui/tests/acceptance/policies/index-test.js
+++ b/ui/tests/acceptance/policies/index-test.js
@ -33,7 +33,7 @@ module('Acceptance | policies/acl', function(hooks) {
  test('it allows deletion of policies with dots in names', async function(assert) {
    const POLICY = 'path "*" { capabilities = ["list"]}';
    let policyName = 'list.policy';
-    await consoleComponent.runCommands([`write sys/policies/acl/${policyName} policy='${POLICY}'`]);
+    await consoleComponent.runCommands([`write sys/policies/acl/${policyName} policy=${btoa(POLICY)}`]);
    await page.visit({ type: 'acl' });
    let policy = page.row.filterBy('name', policyName)[0];
    assert.ok(policy, 'policy is shown in the list');
--- a/ui/tests/acceptance/secrets/backend/kv/secret-test.js
+++ b/ui/tests/acceptance/secrets/backend/kv/secret-test.js
@ -192,17 +192,17 @@ module('Acceptance | secrets/secret/create', function(hooks) {

  test('version 2 with restricted policy still allows creation', async function(assert) {
    let backend = 'kv-v2';
-    const V2_POLICY = `'
+    const V2_POLICY = `
      path "kv-v2/metadata/*" {
        capabilities = ["list"]
      }
      path "kv-v2/data/secret" {
        capabilities = ["create", "read", "update"]
      }
-    '`;
+    `;
    await consoleComponent.runCommands([
      `write sys/mounts/${backend} type=kv options=version=2`,
-      `write sys/policies/acl/kv-v2-degrade policy=${V2_POLICY}`,
+      `write sys/policies/acl/kv-v2-degrade policy=${btoa(V2_POLICY)}`,
      // delete any kv previously written here so that tests can be re-run
      'delete kv-v2/metadata/secret',
      'write -field=client_token auth/token/create policies=kv-v2-degrade',
@ -220,17 +220,17 @@ module('Acceptance | secrets/secret/create', function(hooks) {

  test('version 2 with restricted policy still allows edit', async function(assert) {
    let backend = 'kv-v2';
-    const V2_POLICY = `'
+    const V2_POLICY = `
      path "kv-v2/metadata/*" {
        capabilities = ["list"]
      }
      path "kv-v2/data/secret" {
        capabilities = ["create", "read", "update"]
      }
-    '`;
+    `;
    await consoleComponent.runCommands([
      `write sys/mounts/${backend} type=kv options=version=2`,
-      `write sys/policies/acl/kv-v2-degrade policy=${V2_POLICY}`,
+      `write sys/policies/acl/kv-v2-degrade policy=${btoa(V2_POLICY)}`,
      // delete any kv previously written here so that tests can be re-run
      'delete kv-v2/metadata/secret',
      'write -field=client_token auth/token/create policies=kv-v2-degrade',
@ -255,7 +255,7 @@ module('Acceptance | secrets/secret/create', function(hooks) {
    let paths = [
      '(',
      ')',
-      '"',
+      //'"',
      //"'",
      '!',
      '#',
@ -290,21 +290,23 @@ module('Acceptance | secrets/secret/create', function(hooks) {
    }
  });

-  // the web cli does not handle a single quote in a path, so we test it here via the UI
-  test('creating a secret with a single quote works properly', async function(assert) {
+  // the web cli does not handle a quote as part of a path, so we test it here via the UI
+  test('creating a secret with a single or double quote works properly', async function(assert) {
    await consoleComponent.runCommands('write sys/mounts/kv type=kv');
-    let path = "'some";
-    await listPage.visitRoot({ backend: 'kv' });
-    await listPage.create();
-    await editPage.createSecret(`${path}/2`, 'foo', 'bar');
-    await listPage.visit({ backend: 'kv', id: path });
-    assert.ok(listPage.secrets.filterBy('text', '2')[0], `${path}: secret is displayed properly`);
-    await listPage.secrets.filterBy('text', '2')[0].click();
-    assert.equal(
-      currentRouteName(),
-      'vault.cluster.secrets.backend.show',
-      `${path}: show page renders correctly`
-    );
+    let paths = ["'some", '"some'];
+    for (let path of paths) {
+      await listPage.visitRoot({ backend: 'kv' });
+      await listPage.create();
+      await editPage.createSecret(`${path}/2`, 'foo', 'bar');
+      await listPage.visit({ backend: 'kv', id: path });
+      assert.ok(listPage.secrets.filterBy('text', '2')[0], `${path}: secret is displayed properly`);
+      await listPage.secrets.filterBy('text', '2')[0].click();
+      assert.equal(
+        currentRouteName(),
+        'vault.cluster.secrets.backend.show',
+        `${path}: show page renders correctly`
+      );
+    }
  });

  test('filter clears on nav', async function(assert) {
@ -326,16 +328,16 @@ module('Acceptance | secrets/secret/create', function(hooks) {
  });

  let setupNoRead = async function(backend, canReadMeta = false) {
-    const V2_WRITE_ONLY_POLICY = `'
+    const V2_WRITE_ONLY_POLICY = `
      path "${backend}/+/+" {
        capabilities = ["create", "update", "list"]
      }
      path "${backend}/+" {
        capabilities = ["list"]
      }
-    '`;
+    `;

-    const V2_WRITE_WITH_META_READ_POLICY = `'
+    const V2_WRITE_WITH_META_READ_POLICY = `
      path "${backend}/+/+" {
        capabilities = ["create", "update", "list"]
      }
@ -345,12 +347,12 @@ module('Acceptance | secrets/secret/create', function(hooks) {
      path "${backend}/+" {
        capabilities = ["list"]
      }
-    '`;
-    const V1_WRITE_ONLY_POLICY = `'
+    `;
+    const V1_WRITE_ONLY_POLICY = `
     path "${backend}/+" {
        capabilities = ["create", "update", "list"]
      }
-    '`;
+    `;

    let policy;
    if (backend === 'kv-v2' && canReadMeta) {
@ -364,7 +366,7 @@ module('Acceptance | secrets/secret/create', function(hooks) {
      // disable any kv previously enabled kv
      `delete sys/mounts/${backend}`,
      `write sys/mounts/${backend} type=kv options=version=${backend === 'kv-v2' ? 2 : 1}`,
-      `write sys/policies/acl/${backend} policy=${policy}`,
+      `write sys/policies/acl/${backend} policy=${btoa(policy)}`,
      `write -field=client_token auth/token/create policies=${backend}`,
    ]);

--- a/ui/tests/unit/lib/console-helpers-test.js
+++ b/ui/tests/unit/lib/console-helpers-test.js
@ -26,6 +26,57 @@ module('Unit | Lib | console helpers', function() {
        ],
      ],
    },
+    {
+      name: 'write with space in a value',
+      command: `vault write \
+      auth/ldap/config \
+      url=ldap://ldap.example.com:3268 \
+      binddn="CN=ServiceViewDev,OU=Service Accounts,DC=example,DC=com" \
+      bindpass="xxxxxxxxxxxxxxxxxxxxxxxxxx" \
+      userdn="DC=example,DC=com" \
+      groupdn="DC=example,DC=com" \
+      insecure_tls=true \
+      starttls=false
+      `,
+      expected: [
+        'write',
+        [],
+        'auth/ldap/config',
+        [
+          'url=ldap://ldap.example.com:3268',
+          'binddn=CN=ServiceViewDev,OU=Service Accounts,DC=example,DC=com',
+          'bindpass=xxxxxxxxxxxxxxxxxxxxxxxxxx',
+          'userdn=DC=example,DC=com',
+          'groupdn=DC=example,DC=com',
+          'insecure_tls=true',
+          'starttls=false',
+        ],
+      ],
+    },
+    {
+      name: 'write with double quotes',
+      command: `vault write \
+      auth/token/create \
+      policies="foo"
+      `,
+      expected: ['write', [], 'auth/token/create', ['policies=foo']],
+    },
+    {
+      name: 'write with single quotes',
+      command: `vault write \
+      auth/token/create \
+      policies='foo'
+      `,
+      expected: ['write', [], 'auth/token/create', ['policies=foo']],
+    },
+    {
+      name: 'write with unmatched quotes',
+      command: `vault write \
+      auth/token/create \
+      policies='foo
+      `,
+      expected: ['write', [], 'auth/token/create', ["policies='foo"]],
+    },
    {
      name: 'read with field',
      command: `vault read -field=access_key aws/creds/my-role`,
--- a/ui/yarn.lock
+++ b/ui/yarn.lock
@ -6488,7 +6488,7 @@ decamelize-keys@^1.0.0:
    decamelize "^1.1.0"
    map-obj "^1.0.0"

-decamelize@^1.1.0, decamelize@^1.1.1, decamelize@^1.1.2:
+decamelize@^1.1.0, decamelize@^1.1.1, decamelize@^1.1.2, decamelize@^1.2.0:
  version "1.2.0"
  resolved "https://registry.yarnpkg.com/decamelize/-/decamelize-1.2.0.tgz#f6534d15148269b20352e7bee26f501f9a191290"
  integrity sha1-9lNNFRSCabIDUue+4m9QH5oZEpA=
@ -18775,13 +18775,21 @@ yam@^0.0.24:
    fs-extra "^4.0.2"
    lodash.merge "^4.6.0"

-yargs-parser@^10.0.0, yargs-parser@^10.1.0:
+yargs-parser@^10.1.0:
  version "10.1.0"
  resolved "https://registry.yarnpkg.com/yargs-parser/-/yargs-parser-10.1.0.tgz#7202265b89f7e9e9f2e5765e0fe735a905edbaa8"
  integrity sha512-VCIyR1wJoEBZUqk5PA+oOBF6ypbwh5aNB3I50guxAL/quggdfs4TtNHQrSazFA3fYZ+tEqfs0zIGlv0c/rgjbQ==
  dependencies:
    camelcase "^4.1.0"

+yargs-parser@^13.0.0:
+  version "13.1.0"
+  resolved "https://registry.yarnpkg.com/yargs-parser/-/yargs-parser-13.1.0.tgz#7016b6dd03e28e1418a510e258be4bff5a31138f"
+  integrity sha512-Yq+32PrijHRri0vVKQEm+ys8mbqWjLiwQkMFNXEENutzLPP0bE4Lcd4iA3OQY5HF+GD3xXxf0MEHb8E4/SA3AA==
+  dependencies:
+    camelcase "^5.0.0"
+    decamelize "^1.2.0"
+
 yargs-parser@^5.0.0:
  version "5.0.0"
  resolved "https://registry.yarnpkg.com/yargs-parser/-/yargs-parser-5.0.0.tgz#275ecf0d7ffe05c77e64e7c86e4cd94bf0e1228a"