Skip to content

Commit c699d19

Browse files
jumboduckclaude
andcommitted
feat(evaluate): send a directory of policies as one bundle
--policy may name a directory, and every file below it travels keyed by its path relative to that directory. Nothing is left out by name: what a bundle may hold, and what its modules may import, is for the evaluator that runs it to judge. The published file and byte caps, and an empty directory, are still named here rather than sent to be refused. A policy now comes from this machine only. Fetching one from a URL is on its way out, so this command does not offer it. Slice 6 of kosli-dev/server#6920. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent f765909 commit c699d19

8 files changed

Lines changed: 201 additions & 20 deletions

File tree

‎cmd/kosli/evaluateHelpers.go‎

Lines changed: 76 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"errors"
77
"fmt"
88
"io"
9+
"io/fs"
910
"net/http"
1011
"net/url"
1112
"os"
@@ -36,6 +37,10 @@ const policyMaxBytes = 5 << 20 // 5 MiB
3637
// whenever that schema changes.
3738
const maxServerSideTrails = 100
3839

40+
// maxPolicyBundleFiles mirrors the ceiling the evaluations API publishes on
41+
// the entries in a policy bundle.
42+
const maxPolicyBundleFiles = 100
43+
3944
// serverPolicyMaxBytes mirrors the cap the evaluations API publishes on a
4045
// policy bundle, which counts the names as well as the sources. It is a fifth
4146
// of what a remote --policy read allows, so a policy can be fetched in full
@@ -304,12 +309,7 @@ func runServerEvaluation(out io.Writer, spec serverEvaluation) error {
304309
return err
305310
}
306311

307-
policySource, err := loadPolicy(spec.policyRef)
308-
if err != nil {
309-
return err
310-
}
311-
312-
files, err := policyBundle(spec.policyRef, policySource)
312+
files, err := policyBundle(spec.policyRef)
313313
if err != nil {
314314
return err
315315
}
@@ -437,16 +437,79 @@ func serverVerdict(result *evaluations.Result) *evaluate.Result {
437437
return &evaluate.Result{Allow: result.Allow, Violations: violations}
438438
}
439439

440-
// policyBundle wraps the policy source as the one-file bundle the API takes,
441-
// refusing one too large for it rather than letting the request be rejected.
442-
// The cap counts the names as well as the sources, exactly as the API counts.
443-
func policyBundle(ref string, source []byte) (map[string]string, error) {
444-
key := policyBundleKey(ref)
445-
if size := len(key) + len(source); size > serverPolicyMaxBytes {
440+
// policyBundle reads what --policy names as the bundle the API takes. The caps
441+
// are checked here so an oversized bundle is named as such rather than rejected
442+
// as an opaque 422, and the byte cap counts the names as well as the sources,
443+
// exactly as the API counts.
444+
func policyBundle(ref string) (map[string]string, error) {
445+
files, err := policyBundleFiles(ref)
446+
if err != nil {
447+
return nil, err
448+
}
449+
450+
if len(files) > maxPolicyBundleFiles {
451+
return nil, fmt.Errorf("policy bundle holds %d files, over the limit of %d",
452+
len(files), maxPolicyBundleFiles)
453+
}
454+
size := 0
455+
for name, source := range files {
456+
size += len(name) + len(source)
457+
}
458+
if size > serverPolicyMaxBytes {
446459
return nil, fmt.Errorf("policy bundle is %d bytes, over the %d byte limit",
447460
size, serverPolicyMaxBytes)
448461
}
449-
return map[string]string{key: string(source)}, nil
462+
return files, nil
463+
}
464+
465+
func policyBundleFiles(ref string) (map[string]string, error) {
466+
if !isRemotePolicyRef(ref) {
467+
if info, err := os.Stat(ref); err == nil && info.IsDir() {
468+
return policyDirectory(ref)
469+
}
470+
}
471+
472+
source, err := loadPolicy(ref)
473+
if err != nil {
474+
return nil, err
475+
}
476+
return map[string]string{policyBundleKey(ref): string(source)}, nil
477+
}
478+
479+
// policyDirectory collects every file below root, keyed by its path relative
480+
// to it. Nothing is left out by name: a rule here would refuse bundles the
481+
// evaluator that runs them would have accepted.
482+
func policyDirectory(root string) (map[string]string, error) {
483+
files := map[string]string{}
484+
err := filepath.WalkDir(root, func(path string, entry fs.DirEntry, err error) error {
485+
if err != nil {
486+
return err
487+
}
488+
if entry.IsDir() {
489+
return nil
490+
}
491+
source, err := os.ReadFile(path)
492+
if err != nil {
493+
return fmt.Errorf("failed to read policy file: %w", err)
494+
}
495+
name, err := filepath.Rel(root, path)
496+
if err != nil {
497+
return err
498+
}
499+
// Relative paths reach the API spelled one way, whatever this machine
500+
// spells them with.
501+
files[filepath.ToSlash(name)] = string(source)
502+
return nil
503+
})
504+
if err != nil {
505+
return nil, err
506+
}
507+
if len(files) == 0 {
508+
// The API takes at least one file, so an empty directory is named here
509+
// rather than sent to be refused.
510+
return nil, fmt.Errorf("no file found under %s", root)
511+
}
512+
return files, nil
450513
}
451514

452515
// policyBundleKey names the policy inside the uploaded bundle. Only the base

‎cmd/kosli/evaluatePolicy.go‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ recorded it, and the verdict is printed here.
1818
Name what to evaluate with ` + "`--context trail=<flow>/<trail>`" + `, repeated once per
1919
trail. Trails named in one command are all evaluated at the same instant.
2020
21+
` + "`--policy`" + ` takes a single Rego file or a directory. A directory travels as one
22+
bundle of every file below it, keyed by its path relative to that directory.
23+
2124
Pass ` + "`--control`" + ` to record the outcome as a decision against that control, in the
2225
` + "`--flow`" + ` and ` + "`--trail`" + ` given. The decision is recorded where the policy runs, so
2326
the verdict is never asserted from here. Without ` + "`--control`" + ` nothing is recorded.
@@ -103,7 +106,7 @@ func newEvaluatePolicyCmd(out io.Writer) *cobra.Command {
103106
}
104107

105108
cmd.Flags().StringArrayVar(&o.contexts, "context", []string{}, policyContextFlag)
106-
cmd.Flags().StringVarP(&o.policyRef, "policy", "p", "", "Path or http(s):// URL of a Rego policy to evaluate the trail against.")
109+
cmd.Flags().StringVarP(&o.policyRef, "policy", "p", "", "Path of a Rego policy file, or of a directory sent as one bundle.")
107110
cmd.Flags().StringVar(&o.params, "params", "", policyParamsFlag)
108111
cmd.Flags().StringVarP(&o.output, "output", "o", "table", outputFlag)
109112
cmd.Flags().BoolVar(&o.assert, "assert", false, policyAssertFlag)
@@ -122,6 +125,12 @@ func newEvaluatePolicyCmd(out io.Writer) *cobra.Command {
122125
}
123126

124127
func (o *evaluatePolicyOptions) run(out io.Writer) error {
128+
// Fetching a policy from a URL is on its way out, so this command does not
129+
// offer it, though the older evaluate commands still do.
130+
if isRemotePolicyRef(o.policyRef) {
131+
return fmt.Errorf("--policy takes a file or a directory on this machine, not a URL")
132+
}
133+
125134
trails, err := parseTrailContexts(o.contexts)
126135
if err != nil {
127136
return err

‎cmd/kosli/evaluatePolicy_test.go‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ package main
22

33
import (
44
"fmt"
5+
"os"
6+
"path/filepath"
7+
"sort"
58
"testing"
69
"time"
710

@@ -456,6 +459,92 @@ func (suite *EvaluatePolicyCommandTestSuite) TestAClassifiedFailureIsNotADenial(
456459
require.NotContains(suite.T(), combined, "DENIED")
457460
}
458461

462+
func (suite *EvaluatePolicyCommandTestSuite) TestADirectoryTravelsAsOneBundle() {
463+
server, fake := newFakeEvaluations(suite.T(), verdictAllowed)
464+
465+
_, _, _, _, err := executeCommandC(fmt.Sprintf(
466+
"evaluate policy --context trail=my-flow/my-trail --policy testdata/policies/bundle "+
467+
"--host %s --org test-org --api-token test-token --max-api-retries 0", server.URL))
468+
469+
require.NoError(suite.T(), err)
470+
files := fake.created[0]["policy"].(map[string]interface{})["files"].(map[string]interface{})
471+
require.Equal(suite.T(), []string{"README.md", "lib/helpers.rego", "policy.rego"}, sortedKeys(files),
472+
"keyed by path relative to the directory, and nothing left out by name")
473+
require.Contains(suite.T(), files["policy.rego"], "package policy")
474+
require.Contains(suite.T(), files["lib/helpers.rego"], "package lib.helpers")
475+
}
476+
477+
// The API takes at least one file.
478+
func (suite *EvaluatePolicyCommandTestSuite) TestAnEmptyDirectoryIsRefused() {
479+
directory := suite.T().TempDir()
480+
server, fake := newFakeEvaluations(suite.T(), verdictAllowed)
481+
482+
_, _, _, _, err := executeCommandC(fmt.Sprintf(
483+
"evaluate policy --context trail=my-flow/my-trail --policy %s "+
484+
"--host %s --org test-org --api-token test-token --max-api-retries 0", directory, server.URL))
485+
486+
require.Error(suite.T(), err)
487+
require.Contains(suite.T(), err.Error(), directory)
488+
require.Empty(suite.T(), fake.created)
489+
}
490+
491+
func (suite *EvaluatePolicyCommandTestSuite) TestABundleOverTheCapsIsRefusedWithTheCapNamed() {
492+
for _, test := range []struct {
493+
name string
494+
build func(string)
495+
says string
496+
}{
497+
{"too many files", func(directory string) {
498+
for i := 0; i <= maxPolicyBundleFiles; i++ {
499+
require.NoError(suite.T(), os.WriteFile(
500+
filepath.Join(directory, fmt.Sprintf("policy-%d.rego", i)),
501+
[]byte("package policy\n"), 0644))
502+
}
503+
}, fmt.Sprintf("%d", maxPolicyBundleFiles)},
504+
{"too many bytes", func(directory string) {
505+
source := make([]byte, serverPolicyMaxBytes+1)
506+
for i := range source {
507+
source[i] = 'a'
508+
}
509+
require.NoError(suite.T(), os.WriteFile(
510+
filepath.Join(directory, "policy.rego"), source, 0644))
511+
}, fmt.Sprintf("%d", serverPolicyMaxBytes)},
512+
} {
513+
suite.Run(test.name, func() {
514+
directory := suite.T().TempDir()
515+
test.build(directory)
516+
server, fake := newFakeEvaluations(suite.T(), verdictAllowed)
517+
518+
_, _, _, _, err := executeCommandC(fmt.Sprintf(
519+
"evaluate policy --context trail=my-flow/my-trail --policy %s "+
520+
"--host %s --org test-org --api-token test-token --max-api-retries 0",
521+
directory, server.URL))
522+
523+
require.Error(suite.T(), err)
524+
require.Contains(suite.T(), err.Error(), test.says)
525+
require.Empty(suite.T(), fake.created, "nothing is sent")
526+
})
527+
}
528+
}
529+
530+
// A policy comes from the machine that runs the command.
531+
func (suite *EvaluatePolicyCommandTestSuite) TestARemotePolicyIsRefused() {
532+
for _, ref := range []string{"http://policies.example.com/pr.rego", "https://policies.example.com/pr.rego"} {
533+
suite.Run(ref, func() {
534+
server, fake := newFakeEvaluations(suite.T(), verdictAllowed)
535+
536+
_, combined, _, _, err := executeCommandC(fmt.Sprintf(
537+
"evaluate policy --context trail=my-flow/my-trail --policy %s "+
538+
"--host %s --org test-org --api-token test-token --max-api-retries 0", ref, server.URL))
539+
540+
require.Error(suite.T(), err)
541+
require.Contains(suite.T(), err.Error(), "--policy")
542+
require.Empty(suite.T(), fake.created, "nothing is sent")
543+
require.NotContains(suite.T(), combined, "RESULT")
544+
})
545+
}
546+
}
547+
459548
func (suite *EvaluatePolicyCommandTestSuite) TestADryRunSendsNothing() {
460549
server, fake := newFakeEvaluations(suite.T(), verdictAllowed)
461550

@@ -470,6 +559,15 @@ func (suite *EvaluatePolicyCommandTestSuite) TestADryRunSendsNothing() {
470559
require.NotContains(suite.T(), combined, "RESULT")
471560
}
472561

562+
func sortedKeys(files map[string]interface{}) []string {
563+
keys := make([]string, 0, len(files))
564+
for key := range files {
565+
keys = append(keys, key)
566+
}
567+
sort.Strings(keys)
568+
return keys
569+
}
570+
473571
func TestEvaluatePolicyCommandTestSuite(t *testing.T) {
474572
suite.Run(t, new(EvaluatePolicyCommandTestSuite))
475573
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
The bundle's own notes, which are not a policy and do not travel with it.
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
package lib.helpers
2+
3+
always_true := true
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
package policy
2+
3+
import data.lib.helpers
4+
5+
allow := helpers.always_true

‎docs/handover/6920-evaluate-an-inline-policy-and-record-its-decis.md‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ Server-side evaluation reaches the CLI today only through the hidden `--server-s
2222
- A denial is a decision, recorded as non-compliant with its violations. A policy that cannot run is not: it records nothing and must never print a denial.
2323
- Destination flags without `--control` are refused, and a control or a destination the caller cannot write to is refused before the evaluation is queued.
2424
- The command always waits for a terminal status; `--assert` exits non-zero on denial and changes nothing else.
25-
- `--policy` accepts a single file or a directory, within the published bundle caps.
25+
- `--policy` accepts a single file or a directory on this machine, within the published bundle caps. A URL is refused.
2626
- Output and `--output json` match `kosli evaluate trail`, so a caller switching commands does not re-parse.
2727
- An organisation without the server-side evaluation entitlement is refused in words that name it.
2828
- `--name` defaults to `<control>-decision`, so the common case names only the control.
@@ -42,7 +42,7 @@ Transcribed from [docs/plans/6920-evaluate-policy.md](../plans/6920-evaluate-pol
4242
- [x] Slice 3 — `--context` names what is evaluated, and is always required.
4343
- [x] Slice 4 — `--control` records a decision, with `--flow` and `--trail` as its destination.
4444
- [x] Slice 5 — refusals travel in the API's own words, and a classified failure is never a denial.
45-
- [ ] Slice 6 — a directory of policy files as one bundle, with the caps refused here.
45+
- [x] Slice 6 — a directory of policy files as one bundle, with the caps refused here.
4646
- [ ] Slice 7 — help text, docs, changelog, lint, full test run, and a staging check against an entitled organisation.
4747

4848
---
@@ -60,6 +60,8 @@ Transcribed from [docs/plans/6920-evaluate-policy.md](../plans/6920-evaluate-pol
6060
- The command declares its own options rather than inheriting the evaluate commands' shared ones, because four of those flags have no meaning here and inheriting them only to hide them is how two commands drift apart.
6161
- Asserting is opt-in on this command and the default is silent, which is the reverse of `evaluate trail`. A command that records a decision should not fail a pipeline unless the caller asked it to, and the flag that asks is the one the tutorial already publishes.
6262
- What is evaluated and where a decision lands are named separately: `--context` is the only way to say what to evaluate and is always required, while `--flow` and `--trail` name the destination alone. Neither is refused for being present without `--control`, because a pipeline sets them as environment variables for every command it runs, and refusing them would refuse an ordinary run that asked for no decision. The ticket's example predates this split.
63+
- A policy comes from the machine that runs the command: this command does not fetch one from a URL, though the older evaluate commands do, because that way of naming a policy is on its way out and a new command should not take it on.
64+
- A directory of policy files travels whole, with nothing left out by name and nothing here reading the modules. What a bundle may hold, and what its modules may import, is for the evaluator that runs it to judge; a rule here would refuse bundles the evaluator would have accepted, and would go stale as the evaluator changes. Only the published caps and an empty directory are refused here, because those the caller can act on before sending.
6365
- Refusals are passed on as the API worded them rather than being classified here, and the slice that was to give each case a sentence of its own was cut back to two tests. A list of cases in the CLI would go stale against the server that writes them, and the one thing that must not vary — a failed policy never reading as a denial — is pinned by a test instead.
6466
- The destination is read as a resolved value rather than as a flag the caller typed, so `KOSLI_FLOW` and `KOSLI_TRAIL` satisfy `--control` exactly as the flags do.
6567
- The first cut of the command is synchronous only, and `--sync` is not offered: with nothing to opt into, the flag would name the one behaviour there is. A command whose purpose is recording a decision should not return before the decision exists. An asynchronous mode, and the `--sync` flag that would pair with it, belong to a later ticket if anyone asks for them.
@@ -76,5 +78,5 @@ Transcribed from [docs/plans/6920-evaluate-policy.md](../plans/6920-evaluate-pol
7678

7779
## Next Steps
7880

79-
- [ ] Slice 6: a directory of policy files as one bundle, with the caps refused here.
81+
- [ ] Slice 7: help text, docs, changelog, the full integration run, and a staging check against an entitled organisation.
8082
- [ ] Check what the create endpoint refuses for each destination failure, against staging, so Slice 5's messages are written from real answers rather than guessed.

‎docs/plans/6920-evaluate-policy.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ The evaluation resource gains `decision_attestation_id`: the id of the decision
7676
| Flag | Required | Meaning |
7777
|---|---|---|
7878
| `--context` | yes | Repeatable `trail=<flow>/<trail>`. What is evaluated, all of it at one instant. |
79-
| `--policy`, `-p` | yes | A `.rego` file, a directory, or an `http(s)://` URL. |
79+
| `--policy`, `-p` | yes | A `.rego` file or a directory on this machine. |
8080
| `--params` | no | Inline JSON or `@file.json`, unchanged, read by the policy as `data.params`. |
8181
| `--control` | no | The control the decision answers. Present, a decision is recorded; absent, nothing is. |
8282
| `--flow`, `-f` | with `--control` | Flow the decision is recorded in. |
@@ -136,7 +136,7 @@ The ticket asks for denial, a broken policy and a fault of ours to be three dist
136136

137137
### 4.6 A directory of policy files
138138

139-
`--policy` pointing at a directory uploads every file below it as one bundle, keyed by path relative to that directory, within the published 100-file and 1 MiB caps. Paths stay inside the bundle. A single file keeps today's behaviour: one entry named after the file, no extension imposed.
139+
`--policy` pointing at a directory uploads every file below it as one bundle, keyed by path relative to that directory, within the published 100-file and 1 MiB caps. Nothing is left out by name, and nothing here reads the modules: what a bundle may hold, and what its modules may import, is the evaluator's to judge, and a rule here would refuse bundles the evaluator would have accepted. An empty directory is named here, because the API takes at least one file. A single file keeps today's behaviour: one entry named after the file, no extension imposed. A URL is refused: fetching a policy from one is on its way out, so this command never offers it.
140140

141141
### 4.7 `--context`
142142

@@ -200,7 +200,7 @@ Deliberately shallow. A refusal is reported as the status the API answered with
200200

201201
### Slice 6: a directory of policy files
202202

203-
Relative keys, the file and byte caps refused here with the cap named, paths that would climb out of the bundle refused.
203+
Relative keys, the file and byte caps refused here with the cap named, an empty directory named here, the bundle's contents left for the evaluator to judge, and a URL refused.
204204

205205
### Slice 7: wrap-up
206206

0 commit comments

Comments
 (0)