Repository: oclif/core
Pull request #712 (2023-06-15 14:40:32)

fix: improve flag validation
@W-13157931@

improves flag validation to prevent a flag being read as a flag value

note: I was unable to get this linked and building through my cli. I took a TDD approach instead and wrote the UTs first, and then fixed them


Patch:
diff --git a/src/parser/parse.ts b/src/parser/parse.ts
index ba6ceab..dff54b7 100644
--- a/src/parser/parse.ts
+++ b/src/parser/parse.ts
@@ -86,7 +86,7 @@ export class Parser<T extends ParserInput, TFlags extends OutputFlags<T['flags']
   public async parse(): Promise<ParserOutput<TFlags, BFlags, TArgs>> {
     this._debugInput()
 
-    const findLongFlag = (arg: string) => {
+    const findLongFlag = (arg: string):string | undefined => {
       const name = arg.slice(2)
       if (this.input.flags[name]) {
         return name
@@ -102,12 +102,12 @@ export class Parser<T extends ParserInput, TFlags extends OutputFlags<T['flags']
       }
     }
 
-    const findShortFlag = ([_, char]: string) => {
+    const findShortFlag = ([_, char]: string):string | undefined => {
       if (this.flagAliases[char]) {
         return this.flagAliases[char].name
       }
 
-      return Object.keys(this.input.flags).find(k => this.input.flags[k].char === char)
+      return Object.keys(this.input.flags).find(k => (this.input.flags[k].char === char && char !== undefined && this.input.flags[k].char !== undefined))
     }
 
     const parseFlag = (arg: string): boolean => {
@@ -133,12 +133,13 @@ export class Parser<T extends ParserInput, TFlags extends OutputFlags<T['flags']
       const flag = this.input.flags[name]
       if (flag.type === 'option') {
         this.currentFlag = flag
-        const input = long || arg.length < 3 ? this.argv.shift() : arg.slice(arg[2] === '=' ? 3 : 2)
-        if (typeof input !== 'string') {
+        const value = long || arg.length < 3 ? this.argv.shift()  : arg.slice(arg[2] === '=' ? 3 : 2)
+        // if the value ends up being one of the command's flags, the user didn't provide an input
+        if (typeof value !== 'string' || this.input.flags[findLongFlag(value) as string] || this.input.flags[findShortFlag(value) as string]) {
           throw new CLIError(`Flag --${name} expects a value`)
         }
 
-        this.raw.push({type: 'flag', flag: flag.name, input})
+        this.raw.push({type: 'flag', flag: flag.name, input: value})
       } else {
         this.raw.push({type: 'flag', flag: flag.name, input: arg})
         // push the rest of the short characters back on the stack


Test patch:
diff --git a/test/parser/parse.test.ts b/test/parser/parse.test.ts
index aa9246d..75d1983 100644
--- a/test/parser/parse.test.ts
+++ b/test/parser/parse.test.ts
@@ -84,6 +84,60 @@ describe('parse', () => {
         expect(Boolean(out.flags.myflag2)).to.equal(true)
       })
 
+      it('throws error when no value provided to required flag', async () => {
+        try {
+          await parse(['--myflag', '--second', 'value'], {
+            flags: {myflag: Flags.string({required: true}), second: Flags.string()},
+          })
+          assert.fail('should have thrown')
+        } catch (error) {
+          expect((error as CLIError).message).to.include('Flag --myflag expects a value')
+        }
+      })
+
+      it('throws error when no value provided to required flag before a short char flag', async () => {
+        try {
+          await parse(['--myflag', '-s', 'value'], {
+            flags: {myflag: Flags.string({required: true}), second: Flags.string({char: 's'})},
+          })
+          assert.fail('should have thrown')
+        } catch (error) {
+          expect((error as CLIError).message).to.include('Flag --myflag expects a value')
+        }
+      })
+
+      it('doesn\'t throw when boolean flag passed', async () => {
+        const out =   await parse(['--myflag', '--second', 'value'], {
+          flags: {myflag: Flags.boolean(), second: Flags.string()},
+        })
+        expect(out.flags.myflag).to.be.true
+        expect(out.flags.second).to.equal('value')
+      })
+
+      it('doesn\'t throw when negative number passed', async () => {
+        const out =   await parse(['--myflag', '-s', '-9'], {
+          flags: {myflag: Flags.boolean(), second: Flags.integer({char: 's'})},
+        })
+        expect(out.flags.myflag).to.be.true
+        expect(out.flags.second).to.equal(-9)
+      })
+
+      it('doesn\'t throw when boolean short char is passed', async () => {
+        const out =   await parse(['--myflag', '-s', 'value'], {
+          flags: {myflag: Flags.boolean(), second: Flags.string({char: 's'})},
+        })
+        expect(out.flags.myflag).to.be.true
+        expect(out.flags.second).to.equal('value')
+      })
+
+      it('doesn\'t throw when  short char is passed as a string value', async () => {
+        const out =   await parse(['--myflag', '\'-s\'', '-s', 'value'], {
+          flags: {myflag: Flags.string(), second: Flags.string({char: 's'})},
+        })
+        expect(out.flags.myflag).to.equal('\'-s\'')
+        expect(out.flags.second).to.equal('value')
+      })
+
       it('parses short flags', async () => {
         const out = await parse(['-mf'], {
           flags: {