Closed Bug 1349596 Opened 7 years ago Closed 7 years ago

Recipes with normandy.plugins doesn't execute when condition is true

Categories

(Shield :: General, defect)

defect
Not set
normal

Tracking

(firefox53 verified)

VERIFIED FIXED
Tracking Status
firefox53 --- verified

People

(Reporter: aflorinescu, Assigned: osmose)

References

(Blocks 1 open bug)

Details

[Preconditions:]

1. Obtain a copy of Firefox with the SHIELD recipe client system add-on installed. You can check about:support to ensure that you have it.
2. Set the extensions.shield-recipe-client.dev_mode preference to true to run recipes immediately on startup.
3. Set the extensions.shield-recipe-client.logging.level preference to 0 to enable more logging.
4. Set the security.content.signature.root_hash preference to DB:74:CE:58:E4:F9:D0:9E:E0:42:36:BE:6C:C5:C4:F6:6A:E7:74:7D:C0:21:42:7A:03:BC:2F:57:0C:8B:9B:90.
5. Set the preference value for extensions.shield-recipe-client.api_url set to https://normandy.stage.mozaws.net/api/v1

[Steps:]
1. Open Admin Interface. 
2. Create a default heartbeat type recipe.
3. Set a filter: normandy.plugins['Shockwave Flash'] 
4. Open a FF with a profile containing the preconditions and Flash plugin.
5. Restart FF browser

[Actual Result:}
The recipe is fetched, but not executed.

[Expected Result:]
The heartbeat is executed, as the condition should return true.


[Note:] 
The filter syntax  normandy.plugins['Shockwave Flash'] is taken from http://normandy.readthedocs.io/en/latest/user/filter_expressions.html / Examples.
Mike, we need some clarification on how the filter syntax for normandy.plugins functions? For example, if I'd want to get a filter for plugin "OpenH264 Video Codec provided by Cisco Systems, Inc.", how do I identify the correct syntax for the filter in this case?
Flags: needinfo?(mkelly)
(In reply to Adrian Florinescu [:AdrianSV] from comment #1)
> Mike, we need some clarification on how the filter syntax for
> normandy.plugins functions? For example, if I'd want to get a filter for
> plugin "OpenH264 Video Codec provided by Cisco Systems, Inc.", how do I
> identify the correct syntax for the filter in this case?


The name on about:plugins is the string to use in the filter. So, for example, that string you got is the correct name for that plugin, and the filter expression for it would be:

> normandy.plugins["OpenH264 Video Codec provided by Cisco Systems, Inc."]

What's surprising to me about this bug is that you were able to save a recipe with a filter expression like that. I actually got a validation error when trying to save a recipe with the filter expression: 

> normandy.plugins['Shockwave Flash'] 

I disabled validation locally to get past this, and once I got past that, the filter expression was correctly able to detect that I had flash and the openh264 codec installed. After uninstalling Flash and restarting Firefox, the filter was able to detect that I didn't have it installed and did not execute the recipe.

Can you confirm that you're able to save a recipe with that filter expression on staging, and link to it? In the meantime, I've put up https://github.com/mozilla/normandy/pull/629, which should fix the validation issue.
Flags: needinfo?(mkelly)
(In reply to Michael Kelly [:mkelly,:Osmose] from comment #2)

> What's surprising to me about this bug is that you were able to save a
> recipe with a filter expression like that. I actually got a validation error
> when trying to save a recipe with the filter expression: 
> 
> > normandy.plugins['Shockwave Flash'] 

I get validation error now for both expressions:

normandy.plugins['Shockwave Flash'] 
normandy.plugins["Shockwave Flash"]
Commits pushed to master at https://github.com/mozilla/normandy

https://github.com/mozilla/normandy/commit/24459c8c5ad19f3297efa1d443dbbebbe0a660a6
Bug 1349596: Validate JEXL on the server instead of the client.

Validating JEXL on the client isn't ideal, because the JEXL library doesn't
have a public API for checking if a statement is valid without evaluating
it, which means certain valid expressions won't pass validation if they
reference data in the context that we don't provide in the admin interface.

PyJEXL is a Python library that parses and evaluates JEXL, and it includes
a validation API that is more suitable for our validation.

https://github.com/mozilla/normandy/commit/334d902aa047c61048782f580909df200a58156a
Merge pull request #629 from Osmose/pyjexl-validation

Bug 1349596: Validate JEXL on the server instead of the client.
Adrian: Were you able to verify that plugin filters now work as expected?
Assignee: nobody → mkelly
Flags: needinfo?(adrian.florinescu)
(In reply to Michael Kelly [:mkelly,:Osmose] from comment #5)
> Adrian: Were you able to verify that plugin filters now work as expected?
Yes, could you please mark the bug as resolved so we can mark it as verified?
Flags: needinfo?(adrian.florinescu) → needinfo?(mkelly)
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: needinfo?(mkelly)
Resolution: --- → FIXED
Verified as fixed on Beta53 across Ubuntu 16.04, Win10, OSX 10.12.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.