Setup Regal (#559)

* added custom config

* first lint fix

* Revert "first lint fix"

This reverts commit d949e4b.

* changed CI and makefile command

* switched OPA to regal in CI

* add lint check rule to makefile

* renaming on the way, added a readme

* compiling

* moved buildChainAccount to entity query lib

* desactivated some rules - compiled and lint

* test passing - lint check seem ok

* format and re-added format check for rego

* re-setup opa formatter in CI

* rerun format

* annotate 'evaluate' rule

* removed lint rules for no entrypoint

* format config.yaml

* reactivated linter on full rego, moved main so that it's recognized in engine

* remove messy-incr-rule

* good naming

* activated strict syntax check with OPA native

* added syntax check to CI, updated linter config

* moved config to root

* directory match package structure rule

* renamed contains operator

* removed unused imports

* readme update

* format readme

* typo

* add a test section to readme

* fixed comment in regal config

* renamed makefile

* correct config argument

* enabled camelCase in tests and for variables and packages

* corrected usageof armory.testData

* renamed buildIntent.. to intentTo

* ignore build files

* improved comments in regal config

.github/workflows/policy-engine.yml

@@ -120,14 +120,23 @@ jobs:
       - name: Check out repository code
         uses: actions/checkout@v4
 
+      - name: Setup Regal
+        uses: StyraInc/setup-regal@v1.0.0
+
       - name: Setup OPA
         uses: open-policy-agent/setup-opa@v2
         with:
           version: latest
 
-      - name: Code format
+      - name: Format
         run: make policy-engine/rego/format/check
 
+      - name: Syntax
+        run: make policy-engine/rego/syntax/check
+
+      - name: Lint
+        run: make policy-engine/rego/lint/check REGAL_FORMAT=github
+
       - name: Test
         run: make policy-engine/rego/test
 

.regal/config.yaml

@@ -0,0 +1,72 @@
+ignore:
+  # Ignore userOperation files as functionality is not certain yet.
+  # Ignore dist and rego-build directories as they don't need linting.
+  files:
+    - '*userOperation*'
+    - 'dist/**'
+    - 'rego-build/**'
+rules:
+  style:
+    # This rule is disabled in tests to let us paste in big JSON payloads without having to worry about line length.
+    rule-length:
+      level: error
+      ignore:
+        files:
+          - '*_test.rego'
+    line-length:
+      level: error
+      ignore:
+        files:
+          - '*_test.rego'
+    # Signal TODO comments to the PR reviewer
+    todo-comment:
+      level: warning
+    # We use camel case to comply with our naming conventions
+    prefer-snake-case:
+      level: ignore
+    # In the future, it can be beneficial to re-activate this rule to ensure one source of truth for data and input shape.
+    # It is already done for "data" part, through 'armory.entities' package. "input" is still accessed on the fly when needed.
+    # If we do so, then we can ignore this rule only in querying packages.
+    external-reference:
+      level: ignore
+  custom:
+    naming-convention:
+      level: error
+      ignore:
+        # Ignore the naming convention for 'policy' files - Our policy definition can't be camelCased: permit[{...}].
+        files:
+          - '*_policies*.rego'
+      conventions:
+        # We use camelCase to comply with our monorepo naming conventions
+        # OPA test runner requires test rules to start with "test_", which is not camelCase.
+        # Regal naming conventions applies to all files, we can't subset them by file groups
+        # So we tolerate both standards in the same naming convetion, only for 'rules'.
+        - pattern: '^(test_)?[a-z]+([A-Z][a-z0-9]+)*$'
+          targets:
+            - rule
+          description: 'Use camelCase or test_camelCase for rules'
+        - pattern: '^[a-z]+([A-Z][a-z0-9]+)*$'
+          targets:
+            - function
+          description: 'Use camelCase for all functions'
+        - pattern: ^([a-z][a-z0-9]*([A-Z][a-z0-9]+)*|\$\d+)$
+          targets:
+            - variable
+          description: 'Use camelCase for variables or "$number" for unused variables'
+        - pattern: '^[a-z]+([A-Z][a-z0-9]+)*(\.[a-z]+([A-Z][a-z0-9]+)*)*$'
+          targets:
+            - package
+          description: 'Use camelCase.camelCase... for packages'
+  # Future improvement: remove ignore for test files.
+  # It would be better to strictly split test and production code in different packages to make sure test code is never hit by production one
+  idiomatic:
+    directory-package-mismatch:
+      level: error
+      ignore:
+        files:
+          - '*_test.rego'
+  # Disabling this rule allows to call tested function from the test file without importing functionality package.
+  # Related to comment above, this ignore can be removed if we split test and production code in different packages.
+  testing:
+    test-outside-test-package:
+      level: ignore

apps/policy-engine/Makefile

@@ -2,6 +2,7 @@ POLICY_ENGINE_PROJECT_NAME := policy-engine
 POLICY_ENGINE_PROJECT_DIR := ./apps/policy-engine
 POLICY_ENGINE_DATABASE_SCHEMA := ${POLICY_ENGINE_PROJECT_DIR}/src/shared/module/persistence/schema/schema.prisma
 POLICY_ENGINE_REGO_DIST = ./dist/rego
+REGAL_FORMAT ?= pretty
 
 # === Start ===
 
@@ -158,6 +159,22 @@ policy-engine/rego/format/check:
 	opa fmt \
 		--fail ${POLICY_ENGINE_PROJECT_DIR}/src/resource/open-policy-agent/rego
 
+policy-engine/rego/syntax/check:
+	opa check \
+		--strict \
+		${POLICY_ENGINE_PROJECT_DIR}/src/resource/open-policy-agent/rego
+
+policy-engine/rego/lint/fix:
+	regal fix \
+		${POLICY_ENGINE_PROJECT_DIR}/src/resource/open-policy-agent/rego \
+		--config-file .regal/config.yaml
+
+policy-engine/rego/lint/check:
+	regal lint \
+		--format ${REGAL_FORMAT} \
+		${POLICY_ENGINE_PROJECT_DIR}/src/resource/open-policy-agent/rego \
+		--config-file .regal/config.yaml
+
 policy-engine/rego/test/watch:
 	make policy-engine/rego/test ARGS=--watch
 

apps/policy-engine/src/open-policy-agent/core/util/__test__/unit/rego-transpiler.util.spec.ts

@@ -42,7 +42,7 @@ describe('transpileCriterion', () => {
       criterion: Criterion.CHECK_NONCE_EXISTS,
       args: null
     }
-    expect(transpileCriterion(item)).toEqual(Criterion.CHECK_NONCE_EXISTS)
+    expect(transpileCriterion(item)).toEqual(`criteria.${Criterion.CHECK_NONCE_EXISTS}`)
   })
 
   it('returns criterion if args is an array of strings', () => {
@@ -51,7 +51,7 @@ describe('transpileCriterion', () => {
       args: ['0x123', '0x456']
     }
 
-    expect(transpileCriterion(item)).toEqual(`${Criterion.CHECK_ACCOUNT_ADDRESS}({"0x123", "0x456"})`)
+    expect(transpileCriterion(item)).toEqual(`criteria.${Criterion.CHECK_ACCOUNT_ADDRESS}({"0x123", "0x456"})`)
   })
 
   it('returns criterion if args is an array of objects', () => {
@@ -61,7 +61,7 @@ describe('transpileCriterion', () => {
     }
 
     expect(transpileCriterion(item)).toEqual(
-      `${Criterion.CHECK_ERC1155_TRANSFERS}([${item.args.map((el) => JSON.stringify(el)).join(', ')}])`
+      `criteria.${Criterion.CHECK_ERC1155_TRANSFERS}([${item.args.map((el) => JSON.stringify(el)).join(', ')}])`
     )
   })
 
@@ -74,7 +74,7 @@ describe('transpileCriterion', () => {
       }
     }
 
-    expect(transpileCriterion(item)).toEqual(`${Criterion.CHECK_INTENT_AMOUNT}(${JSON.stringify(item.args)})`)
+    expect(transpileCriterion(item)).toEqual(`criteria.${Criterion.CHECK_INTENT_AMOUNT}(${JSON.stringify(item.args)})`)
   })
 
   it('returns approvals criterion', () => {
@@ -91,7 +91,7 @@ describe('transpileCriterion', () => {
     }
 
     expect(transpileCriterion(item)).toEqual(
-      `approvals = ${Criterion.CHECK_APPROVALS}([${item.args.map((el) => JSON.stringify(el)).join(', ')}])`
+      `approvals = criteria.${Criterion.CHECK_APPROVALS}([${item.args.map((el) => JSON.stringify(el)).join(', ')}])`
     )
   })
 })

apps/policy-engine/src/open-policy-agent/core/util/__test__/unit/wasm-build.util.spec.ts

@@ -73,8 +73,8 @@ describe('copyRegoCore', () => {
         destination: path
       })
 
-      expect(existsSync(`${path}/criteria`)).toEqual(true)
-      expect(existsSync(`${path}/main.rego`)).toEqual(true)
+      expect(existsSync(`${path}/armory`)).toEqual(true)
+      expect(existsSync(`${path}/main/evaluate.rego`)).toEqual(true)
     })
   })
 
@@ -128,7 +128,7 @@ describe('unzip', () => {
       // gzip file. That's why the `path` exists within the dist directory as
       // well.
       // See https://www.openpolicyagent.org/docs/latest/management-bundles/#bundle-file-format
-      expect(existsSync(`${distDirectory}/${path}/rego/main.rego`)).toEqual(true)
+      expect(existsSync(`${distDirectory}/${path}/rego/main/evaluate.rego`)).toEqual(true)
     })
   })
 })

apps/policy-engine/src/open-policy-agent/core/util/rego-transpiler.util.ts

@@ -15,22 +15,20 @@ export const transpileCriterion = (item: PolicyCriterion) => {
   if (!isEmpty(args)) {
     if (Array.isArray(args)) {
       if (typeof args[0] === 'string') {
-        return `${criterion}({${args.map((el) => `"${el}"`).join(', ')}})`
+        return `criteria.${criterion}({${args.map((el) => `"${el}"`).join(', ')}})`
       }
 
       if (criterion === Criterion.CHECK_APPROVALS) {
         // TODO: (@wcalderipe, 18/03/24): Explore with team a threat model on
         // string interpolation injection.
-        return `approvals = ${criterion}([${args.map((el) => JSON.stringify(el)).join(', ')}])`
+        return `approvals = criteria.${criterion}([${args.map((el) => JSON.stringify(el)).join(', ')}])`
       }
-
-      return `${criterion}([${args.map((el) => JSON.stringify(el)).join(', ')}])`
+      return `criteria.${criterion}([${args.map((el) => JSON.stringify(el)).join(', ')}])`
     }
-
-    return `${criterion}(${JSON.stringify(args)})`
+    return `criteria.${criterion}(${JSON.stringify(args)})`
   }
 
-  return `${criterion}`
+  return `criteria.${criterion}`
 }
 
 export const transpileReason = (item: Policy & { id: string }) => {

apps/policy-engine/src/resource/open-policy-agent/rego/README.md

@@ -0,0 +1,44 @@
+# Packages
+
+## Main
+
+The implementation of engine general logic. This package is the one were transpiled policy are written
+Transpiled policies then use Armory/criteria functions to evaluate input
+
+## Armory
+
+### Constants
+
+This package contains all constants that are used by production code
+
+### Criteria
+
+Criteria contains the function that build the logic for every transpiled policy.
+**This package should have exactly the same number of files as we have supported criteria**
+
+### Feeds
+
+Criteria that needs to access our feeds should be depending from this package
+
+### Entities
+
+Functions used to query loaded data. It serves as a source of truth to know if something is in data.entities.
+
+- Enforce invariants like lowercasing hex addresses
+- Aggregate data from multiple places in entity in order to build useful relationships
+- build runtime types that depends on entity data result
+
+### Lib
+
+Utils that are not domain specific, like case insensitive comparison or time.
+
+### Test_Data
+
+Values that are specifically used by tests.
+**this shouldn't be imported in production code**
+
+# Tests
+
+Currently tests are not separated in different package. Future improvement should be to strictly separate test and production code
+By moving tests in separate packages.
+We can enforce it by re-enabling `test-outside-test-package` rule in `.regal/config.yaml`