Proper runtsafe scope for commands and detect undeclared vars

synapse/lib/ast.py

@@ -92,6 +92,9 @@ def init(self, core):
         [k.init(core) for k in self.kids]
         self.prepare()
 
+    def validate(self, runt):
+        [k.validate(runt) for k in self.kids]
+
     def prepare(self):
         pass
 
@@ -116,8 +119,7 @@ def __iter__(self):
 
     def getRuntVars(self, runt):
         for kid in self.kids:
-            for name in kid.getRuntVars(runt):
-                yield name
+            yield from kid.getRuntVars(runt)
 
     def isRuntSafe(self, runt):
         return all(k.isRuntSafe(runt) for k in self.kids)
@@ -158,6 +160,7 @@ async def iterNodePaths(self, runt, genr=None):
             subgraph = SubGraph(rules)
 
         self.optimize()
+        self.validate(runt)
 
         # turtles all the way down...
         if genr is None:
@@ -521,18 +524,16 @@ class ForLoop(Oper):
 
     def getRuntVars(self, runt):
 
-        if not self.kids[1].isRuntSafe(runt):
-            return
+        runtsafe = self.kids[1].isRuntSafe(runt)
 
         if isinstance(self.kids[0], VarList):
             for name in self.kids[0].value():
-                yield name
+                yield name, runtsafe
 
         else:
-            yield self.kids[0].value()
+            yield self.kids[0].value(), runtsafe
 
-        for name in self.kids[2].getRuntVars(runt):
-            yield name
+        yield from self.kids[2].getRuntVars(runt)
 
     async def run(self, runt, genr):
 
@@ -720,31 +721,25 @@ async def run(self, runt, genr):
 
         with s_provenance.claim('stormcmd', name=name):
 
-            if runtsafe:
-
-                genr = await pullone(genr)
-
-                argv = await self.kids[1].compute(runt, None)
-                if not await scmd.setArgv(argv):
-                    return
-
-                async for item in scmd.execStormCmd(runt, genr):
-                    yield item
-
-                return
-
-            async def optsgenr():
+            async def genx():
 
                 async for node, path in genr:
-
                     argv = await self.kids[1].compute(runt, path)
                     if not await scmd.setArgv(argv):
                         return
 
                     yield node, path
 
-            scmd = ctor(runt, runtsafe)
-            async for item in scmd.execStormCmd(runt, optsgenr()):
+            # must pull through the genr to get opts set
+            # ( many commands expect self.opts is set at run() )
+            genr = await pullone(genx())
+
+            if runtsafe:
+                argv = await self.kids[1].compute(runt, None)
+                if not await scmd.setArgv(argv):
+                    return
+
+            async for item in scmd.execStormCmd(runt, genr):
                 yield item
 
 class SetVarOper(Oper):
@@ -773,9 +768,9 @@ async def run(self, runt, genr):
             runt.setVar(name, valu)
 
     def getRuntVars(self, runt):
-        if not self.kids[1].isRuntSafe(runt):
-            return
-        yield self.kids[0].value()
+        yield self.kids[0].value(), self.kids[1].isRuntSafe(runt)
+        for k in self.kids:
+            yield from k.getRuntVars(runt)
 
 class SetItemOper(Oper):
     '''
@@ -846,12 +841,9 @@ async def run(self, runt, genr):
             return
 
     def getRuntVars(self, runt):
-
-        if not self.kids[1].isRuntSafe(runt):
-            return
-
+        runtsafe = self.kids[1].isRuntSafe(runt)
         for name in self.kids[0].value():
-            yield name
+            yield name, runtsafe
 
 class VarEvalOper(Oper):
     '''
@@ -1007,7 +999,7 @@ async def yieldFromValu(self, runt, valu):
                     yield node
             return
 
-        if isinstance(valu, (list, tuple)):
+        if isinstance(valu, (list, tuple, set)):
             for item in valu:
                 async for node in self.yieldFromValu(runt, item):
                     yield node
@@ -1022,11 +1014,13 @@ async def yieldFromValu(self, runt, valu):
         if isinstance(valu, s_stormtypes.Query):
             async for node in valu.nodes():
                 yield node
+            return
 
         if isinstance(valu, (s_stormtypes.List, s_stormtypes.Set)):
             for item in valu.valu:
                 async for node in self.yieldFromValu(runt, item):
                     yield node
+            return
 
 class LiftTag(LiftOper):
 
@@ -2288,6 +2282,10 @@ def isRuntSafe(self, runt):
         # an argv query is really just a string, so it's runtsafe.
         return True
 
+    def validate(self, runt):
+        # validation is done by the sub-runtime
+        pass
+
     async def compute(self, runt, path):
         return self.kids[0].text
 
@@ -2392,6 +2390,10 @@ class CallKwargs(CallArgs):
 
 class VarValue(Value, Cond):
 
+    def validate(self, runt):
+        if runt.runtvars.get(self.name) is None:
+            raise s_exc.NoSuchVar(name=self.name)
+
     async def getCondEval(self, runt):
 
         async def cond(node, path):
@@ -2595,6 +2597,13 @@ class Bool(Const):
 
 class EmbedQuery(Const):
 
+    def validate(self, runt):
+        # var scope validation occurs in the sub-runtime
+        pass
+
+    def getRuntVars(self, runt):
+        if 0: yield
+
     async def compute(self, runt, path):
 
         varz = {}
@@ -3336,7 +3345,11 @@ async def realfunc(*args, **kwargs):
             yield node, path
 
     def getRuntVars(self, runt):
-        yield self.kids[0].value()
+        yield (self.kids[0].value(), True)
+
+    def validate(self, runt):
+        # var scope validation occurs in the sub-runtime
+        pass
 
     async def callfunc(self, runt, args, kwargs):
         '''

synapse/lib/node.py

@@ -85,10 +85,15 @@ async def storm(self, runt, text, opts=None, path=None):
             If opts is not None and opts['vars'] is set and path is not None, then values of path vars take precedent
         '''
         query = self.snap.core.getStormQuery(text)
-        with runt.getSubRuntime(query, opts=opts) as subr:
 
-            if path is not None:
-                subr.vars.update(path.vars)
+        if opts is None:
+            opts = {}
+
+        opts.setdefault('vars', {})
+        if path is not None:
+            opts['vars'].update(path.vars)
+
+        with runt.getSubRuntime(query, opts=opts) as subr:
 
             subr.addInput(self)
 

synapse/lib/storm.py

@@ -1088,16 +1088,22 @@ def __init__(self, query, snap, opts=None, user=None, root=None):
         if varz is not None:
             self.vars.update(varz)
 
-        self.runtvars = set()
-        self.runtvars.update(self.vars.keys())
-        self.runtvars.update(self.ctors.keys())
-
-        self.proxies = {}
+        # declare path builtins as non-runtsafe
+        self.runtvars = {
+            'node': False,
+            'path': False,
+        }
 
         # inherit runtsafe vars from our root
         if self.root is not None:
             self.runtvars.update(root.runtvars)
 
+        # all vars/ctors are de-facto runtsafe
+        self.runtvars.update({k: True for k in self.vars.keys()})
+        self.runtvars.update({k: True for k in self.ctors.keys()})
+
+        self.proxies = {}
+
         self._loadRuntVars(query)
 
     async def dyncall(self, iden, todo, gatekeys=()):
@@ -1131,9 +1137,7 @@ async def getTeleProxy(self, url, **opts):
         return prox
 
     def isRuntVar(self, name):
-        if name in self.runtvars:
-            return True
-        return False
+        return bool(self.runtvars.get(name))
 
     async def printf(self, mesg):
         return await self.snap.printf(mesg)
@@ -1240,8 +1244,11 @@ def confirm(self, perms, gateiden=None):
     def _loadRuntVars(self, query):
         # do a quick pass to determine which vars are per-node.
         for oper in query.kids:
-            for name in oper.getRuntVars(self):
-                self.runtvars.add(name)
+            for name, isrunt in oper.getRuntVars(self):
+                # once runtsafe, always runtsafe
+                if self.runtvars.get(name):
+                    continue
+                self.runtvars[name] = isrunt
 
     async def execute(self, genr=None):
         with s_provenance.claim('storm', q=self.query.text, user=self.user.iden):
@@ -1730,27 +1737,22 @@ async def execStormCmd(self, runt, genr):
         text = self.cdef.get('storm')
         query = runt.snap.core.getStormQuery(text)
 
+        cmdopts = s_stormtypes.CmdOpts(self)
+
         opts = {
             'vars': {
+                'cmdopts': cmdopts,
                 'cmdconf': self.cdef.get('cmdconf', {}),
             }
         }
 
-        if self.opts is not None:
-            opts['vars']['cmdopts'] = vars(self.opts)
+        async def genx():
+            async for xnode, xpath in genr:
+                xpath.initframe(initvars={'cmdopts': cmdopts})
+                yield xnode, xpath
 
         with runt.getCmdRuntime(query, opts=opts) as subr:
-
-            async def wrapgenr():
-
-                async for node, path in genr:
-                    # In the event of a non-runtsafe command invocation our
-                    # setArgv() method will be called with an argv computed
-                    # for each node, so we need to remap cmdopts into our path & subr
-                    path.initframe(initvars={'cmdopts': vars(self.opts)})
-                    yield node, path
-
-            async for node, path in subr.execute(genr=wrapgenr()):
+            async for node, path in subr.execute(genr=genx()):
                 path.finiframe()
                 yield node, path
 

synapse/lib/stormtypes.py

@@ -1922,6 +1922,39 @@ async def deref(self, name):
     async def value(self):
         return {await toprim(k): await toprim(v) for (k, v) in self.valu.items()}
 
+@registry.registerType
+class CmdOpts(Dict):
+    '''
+    A dictionary like object that holds a reference to a command options namespace.
+    ( This allows late-evaluation of command arguments rather than forcing capture )
+    '''
+
+    def __iter__(self):
+        valu = vars(self.valu.opts)
+        return valu.items()
+
+    async def __aiter__(self):
+        valu = vars(self.valu.opts)
+        for item in valu.items():
+            yield item
+
+    def __len__(self):
+        valu = vars(self.valu.opts)
+        return len(valu)
+
+    async def setitem(self, name, valu):
+        # due to self.valu.opts potentially being replaced
+        # we disallow setitem() to prevent confusion
+        mesg = 'CmdOpts may not be modified by the runtime'
+        raise s_exc.StormRuntimeError(mesg=mesg, name=name)
+
+    async def deref(self, name):
+        return getattr(self.valu.opts, name, None)
+
+    async def value(self):
+        valu = vars(self.valu.opts)
+        return {await toprim(k): await toprim(v) for (k, v) in valu.items()}
+
 @registry.registerType
 class Set(Prim):
 

synapse/tests/test_cortex.py

@@ -701,7 +701,15 @@ async def test_cortex_pure_cmds(self):
                 'hehe': 'haha',
             },
 
-            'storm': '$foo=$(10) if $cmdopts.domore { [ +#$cmdconf.hehe ] } [ +#$cmdopts.tagname ]',
+            'storm': '''
+                $foo=$(10)
+                if $cmdopts.domore {
+                    [ +#$cmdconf.hehe ]
+                }
+                $lib.print(TAGNAME)
+                $lib.print($cmdopts)
+                [ +#$cmdopts.tagname ]
+            ''',
         }
 
         cdef1 = {
@@ -4260,6 +4268,7 @@ async def test_cortex_storm_lib_dmon_cmds(self):
             # make the dmon blow up
             await core.nodes('''
                 $lib.queue.get(boom).put(hehe)
+                $q = $lib.queue.get(visi)
                 for ($offs, $item) in $q.gets(size=1) { $q.cull($offs) }
             ''')