Skip to content

Commit 80fd958

Browse files
alnrclaude
andcommitted
fix: harden ory validate opl after review
- an empty file is valid; the SDK rejects a zero-length body, so skip the call - use the nil-safe accessor for the result, which is nil on an empty response - always emit the `errors` key in machine-readable formats, so that CI scripts can rely on `jq '.errors | length'` - render the column only together with the line, so that a line-less position cannot be misread as a line number - treat an unknown `--format` as human-readable, matching cmdx itself - do not claim `update opl` validates through the same endpoint Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012UL5u7KWLdu8hzNTNvYq5a
1 parent 2f7fb86 commit 80fd958

2 files changed

Lines changed: 80 additions & 20 deletions

File tree

cmd/cloudx/project/validate_namespace_config.go

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,10 @@ func NewValidateNamespaceConfigCmd() *cobra.Command {
3232
Short: "Validate the syntax of an Ory Permission Language file",
3333
Long: `Validate the syntax of an Ory Permission Language file without applying it.
3434
35-
Ory Network checks the file the same way ` + "`ory update opl`" + ` does before storing it,
36-
but nothing is written to the project. The command exits with a non-zero status
37-
when the file has syntax errors, which makes it usable as a CI/CD check.`,
35+
The file is checked by the Ory Network project's syntax check endpoint; nothing
36+
is written to the project. The command exits with a non-zero status when the
37+
file has syntax errors, which makes it usable as a CI/CD check before running
38+
` + "`ory update opl`" + `.`,
3839
Example: `$ {{ .CommandPath }} --file /path/to/namespace_config.ts
3940
4041
The Ory Permission Language file is valid.
@@ -59,22 +60,33 @@ $ cat namespace_config.ts | {{ .CommandPath }} --file - --format json
5960
return err
6061
}
6162

62-
result, _, err := c.RelationshipAPI.CheckOplSyntax(ctx).Body(string(data)).Execute()
63-
if err != nil {
64-
return cmdx.PrintOpenAPIError(cmd, err)
63+
var errs oplSyntaxErrors
64+
// An empty file has no syntax errors, and the SDK refuses to send a
65+
// zero-length body ("invalid body type text/plain").
66+
if len(data) > 0 {
67+
result, _, err := c.RelationshipAPI.CheckOplSyntax(ctx).Body(string(data)).Execute()
68+
if err != nil {
69+
return cmdx.PrintOpenAPIError(cmd, err)
70+
}
71+
// GetErrors is nil-safe: result is nil if the API replies with an empty body.
72+
errs = oplSyntaxErrors(result.GetErrors())
6573
}
6674

67-
errs := oplSyntaxErrors(result.Errors)
6875
switch outputFormat(cmd) {
69-
case cmdx.FormatDefault, cmdx.FormatTable, cmdx.FormatQuiet:
76+
case cmdx.FormatJSON, cmdx.FormatJSONPretty, cmdx.FormatJSONPath, cmdx.FormatJSONPointer, cmdx.FormatYAML:
77+
cmdx.PrintTable(cmd, errs)
78+
default:
79+
// Everything else (including an unknown value, which cmdx itself
80+
// treats as the default format) is rendered for humans.
81+
source := sourceName(file)
7082
for _, parseErr := range errs {
71-
_, _ = fmt.Fprintln(cmd.ErrOrStderr(), formatParseError(sourceName(file), parseErr))
83+
// Syntax errors are what the command is for, so they are also
84+
// printed with --quiet; only the success message is noise.
85+
_, _ = fmt.Fprintln(cmd.ErrOrStderr(), formatParseError(source, parseErr))
7286
}
7387
if len(errs) == 0 {
7488
_, _ = cmdx.NewLoudOutPrinter(cmd).Println("The Ory Permission Language file is valid.")
7589
}
76-
default:
77-
cmdx.PrintTable(cmd, errs)
7890
}
7991

8092
if len(errs) > 0 {
@@ -128,10 +140,10 @@ func outputFormat(cmd *cobra.Command) cmdx.Format {
128140
// and CI log parsers can pick up the location.
129141
func formatParseError(source string, err cloud.ParseError) string {
130142
location := source
131-
if start := err.Start; start != nil {
132-
if start.Line != nil {
133-
location += ":" + strconv.FormatInt(*start.Line, 10)
134-
}
143+
// The column is only appended together with the line, otherwise it would be
144+
// rendered in the position where readers expect the line number.
145+
if start := err.Start; start != nil && start.Line != nil {
146+
location += ":" + strconv.FormatInt(*start.Line, 10)
135147
if start.Column != nil {
136148
location += ":" + strconv.FormatInt(*start.Column, 10)
137149
}
@@ -165,6 +177,11 @@ func (e oplSyntaxErrors) Table() [][]string {
165177
}
166178

167179
func (e oplSyntaxErrors) Interface() interface{} {
180+
if e == nil {
181+
// The field is omitted when nil, but CI/CD consumers should be able to
182+
// rely on "errors" always being present, e.g. `jq '.errors | length'`.
183+
e = oplSyntaxErrors{}
184+
}
168185
return cloud.CheckOplSyntaxResult{Errors: e}
169186
}
170187

cmd/cloudx/project/validate_namespace_config_test.go

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ func TestValidateNamespaceConfig(t *testing.T) {
3434
validate := func(t *testing.T, exec execFunc) {
3535
stdout, stderr, err := exec(nil, "validate", "opl", "--file", config)
3636
require.NoError(t, err, stderr)
37-
assert.Contains(t, stdout, "valid")
37+
// not just "valid", which is also a substring of "invalid"
38+
assert.Contains(t, stdout, "file is valid")
3839
assert.Empty(t, stderr)
3940
}
4041

@@ -76,30 +77,72 @@ func TestValidateNamespaceConfig(t *testing.T) {
7677
runWithProjectAsFlag(ctx, t, extraProject.Id, validate)
7778
})
7879

80+
t.Run("reports an empty error list as JSON", func(t *testing.T) {
81+
t.Parallel()
82+
83+
config := writeFile(t, validOPL)
84+
validate := func(t *testing.T, exec execFunc) {
85+
stdout, stderr, err := exec(nil, "validate", "opl", "--file", config, "--format", "json")
86+
require.NoError(t, err, stderr)
87+
88+
// the key is always present, so that `jq '.errors | length'` works
89+
errs := gjson.Get(stdout, "errors")
90+
require.True(t, errs.IsArray(), stdout)
91+
assert.Empty(t, errs.Array(), stdout)
92+
}
93+
94+
runWithProjectAsDefault(ctx, t, defaultProject.Id, validate)
95+
})
96+
97+
t.Run("accepts an empty file", func(t *testing.T) {
98+
t.Parallel()
99+
100+
config := writeFile(t, "")
101+
validate := func(t *testing.T, exec execFunc) {
102+
stdout, stderr, err := exec(nil, "validate", "opl", "--file", config)
103+
require.NoError(t, err, stderr)
104+
assert.Contains(t, stdout, "file is valid")
105+
}
106+
107+
runWithProjectAsDefault(ctx, t, defaultProject.Id, validate)
108+
})
109+
79110
t.Run("reads the file from stdin", func(t *testing.T) {
80111
t.Parallel()
81112

82113
validate := func(t *testing.T, exec execFunc) {
83114
stdout, stderr, err := exec(strings.NewReader(validOPL), "validate", "opl", "--file", "-")
84115
require.NoError(t, err, stderr)
85-
assert.Contains(t, stdout, "valid")
116+
assert.Contains(t, stdout, "file is valid")
117+
118+
// errors are reported against "stdin", not against the literal "-"
119+
_, stderr, err = exec(strings.NewReader(invalidOPL), "validate", "opl", "--file", "-")
120+
assert.ErrorIs(t, err, cmdx.ErrNoPrintButFail, stderr)
121+
assert.Contains(t, stderr, "stdin:", stderr)
86122
}
87123

88124
runWithProjectAsDefault(ctx, t, defaultProject.Id, validate)
89125
runWithProjectAsFlag(ctx, t, extraProject.Id, validate)
90126
})
91127

92-
t.Run("prints nothing on success when quiet", func(t *testing.T) {
128+
t.Run("stays quiet on success but still reports syntax errors", func(t *testing.T) {
93129
t.Parallel()
94130

95-
config := writeFile(t, validOPL)
131+
valid, invalid := writeFile(t, validOPL), writeFile(t, invalidOPL)
96132
validate := func(t *testing.T, exec execFunc) {
97-
stdout, stderr, err := exec(nil, "validate", "opl", "--file", config, "--quiet")
133+
stdout, stderr, err := exec(nil, "validate", "opl", "--file", valid, "--quiet")
98134
require.NoError(t, err, stderr)
99135
assert.Empty(t, stdout)
100136
assert.Empty(t, stderr)
137+
138+
// --quiet silences the success message, not the syntax errors
139+
stdout, stderr, err = exec(nil, "validate", "opl", "--file", invalid, "--quiet")
140+
assert.ErrorIs(t, err, cmdx.ErrNoPrintButFail, stderr)
141+
assert.Empty(t, stdout)
142+
assert.Contains(t, stderr, invalid+":", stderr)
101143
}
102144

103145
runWithProjectAsDefault(ctx, t, defaultProject.Id, validate)
146+
runWithProjectAsFlag(ctx, t, extraProject.Id, validate)
104147
})
105148
}

0 commit comments

Comments
 (0)