open-policy-agent/opa

Parser does not respect capabilities file when using `opa eval`

johanfylling opened this issue · 8 comments

Given the policy:

package test

import future.keywords.if

p if {
   true
}

and a modified capabilities file, where we've removed the if keyword:

{
  "builtins": [
    {
      "name": "eq",
      "decl": {
        "args": [
          {
            "type": "any"
          },
          {
            "type": "any"
          }
        ],
        "result": {
          "type": "boolean"
        },
        "type": "function"
      },
      "infix": "="
    }
  ],
  "future_keywords": [
    "contains",
    "every",
    "in"
  ],
  "features": [
    "rule_head_ref_string_prefixes"
  ]
}

When running opa eval -d policy.rego --capabilities capabilities.json -fpretty 'data', we expect the error message:

1 error occurred: policy.rego:3: rego_parse_error: unexpected keyword, must be one of [contains every in]
	import future.keywords.if

but instead we get a successful evaluation:

{
  "test": {
    "p": true
  }
}

Is this a bug, or by design?

The opa build command will respect the passed capabilities file. One could reason that it's only during commands like check and build we want to impose parser limitations on OPA, and the reason for eval taking a --capabilities flag is to inform it about what built-ins are available, not to restrict its parsing.

If this isn't a bug, and should be corrected, this is where the capabilities are dropped.

the reason for eval taking a --capabilities flag is to inform it about what built-ins are available, not to restrict its parsing.

If this was by design it should have been documented, no? Seems like a bug.

Yes, it's a peculiar limitation if by design. And I'd honestly be surprised if anyone expected eval to behave differently than build and check. Especially since it's undocumented.

I know I’m probably the only user of “opa parse”, but I guess it makes sense for that to have a capabilities flag added if that has an impact on parsing?

it makes sense for that to have a capabilities flag added if that has an impact on parsing?

yes, I suppose so. All it'd do with the current set of capabilities we have is to stop it in its tracks and output an error. But it'd probably be a good thing to have it behave as the other commands.

Agreed, but fine to ignore until someone else asks about it.

This issue has been automatically marked as inactive because it has not had any activity in the last 30 days. Although currently inactive, the issue could still be considered and actively worked on in the future. More details about the use-case this issue attempts to address, the value provided by completing it or possible solutions to resolve it would help to prioritize the issue.