feat(core): Add search term information to protos.#3547
Merged
Conversation
github-actions
Bot
added
comp:policy
Contributor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a standardized search capability across the core policy services. By defining a common Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. A query string for search we find, / To leave the simple lists behind. / With protos updated, clean and bright, / The data flows with newfound light. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a new Search message and adds an optional search field to various list request messages across multiple protobuf files to support search functionality. A review comment identifies an inconsistency where the protobuf definition in selectors.proto uses the field name term, while the generated documentation in docs/grpc/index.html refers to it as query. It is recommended to update the protobuf field name to query to maintain consistency.
|
Warning Review limit reached
More reviews will be available in 38 minutes and 51 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (7)
📒 Files selected for processing (16)
📝 WalkthroughWalkthroughThe PR adds a reusable ChangesSearch filter contract
OpenAPI path and schema contract updates
Sequence Diagram(s)None. Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
c-r33d
changed the title
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Signed-off-by: Chris Reed <creed@virtru.com>
Contributor
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
docs/openapi/common/common.openapi.yaml (1)
23-56:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestore the “at least one identifier” constraint on these shared schemas.
After dropping the conditional
requiredbranches, bothcommon.IdFqnIdentifierandcommon.IdNameIdentifiernow accept{}. That weakens every request that reuses them and moves a shared validation rule into each individual service.Possible fix
common.IdFqnIdentifier: type: object + oneOf: + - required: + - id + - required: + - fqn properties: id: type: string @@ common.IdNameIdentifier: type: object + oneOf: + - required: + - id + - required: + - name properties: id: type: string🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/openapi/common/common.openapi.yaml` around lines 23 - 56, The shared schemas common.IdFqnIdentifier and common.IdNameIdentifier currently allow an empty object because the conditional required branches were removed; restore the "at least one identifier" constraint by adding a oneOf to each schema that enforces either id is present or the other identifier is present (for common.IdFqnIdentifier oneOf: required ['id'] OR required ['fqn']; for common.IdNameIdentifier oneOf: required ['id'] OR required ['name']), keeping additionalProperties: false and existing property definitions intact so consumers cannot pass {}.docs/openapi/authorization/authorization.openapi.yaml (1)
799-808:⚠️ Potential issue | 🟠 MajorFix
policy.KasPublicKey.algguard usingnot.enum: [0]with$ref: policy.KasPublicKeyAlgEnum
policy.KasPublicKeyAlgEnumis atype: stringenum that includesKAS_PUBLIC_KEY_ALG_ENUM_UNSPECIFIED, but thealgschema excludes numeric0(not: enum: - 0) while still$ref’ing the string enum—so the guard no longer excludes the intended unspecified value and widens the accepted contract. In theseopenapi: 3.1.0specs, siblingnotwith$refis applied, so this impacts validation.
docs/openapi/authorization/authorization.openapi.yaml(799-808)docs/openapi/authorization/v2/authorization.openapi.yamldocs/openapi/policy/actions/actions.openapi.yaml- Also present in:
docs/openapi/policy/attributes/attributes.openapi.yaml,docs/openapi/policy/kasregistry/key_access_server_registry.openapi.yaml,docs/openapi/policy/namespaces/namespaces.openapi.yaml,docs/openapi/policy/objects.openapi.yaml,docs/openapi/policy/obligations/obligations.openapi.yaml,docs/openapi/policy/registeredresources/registered_resources.openapi.yaml,docs/openapi/policy/resourcemapping/resource_mapping.openapi.yaml,docs/openapi/policy/subjectmapping/subject_mapping.openapi.yaml,docs/openapi/policy/unsafe/unsafe.openapi.yaml🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/openapi/authorization/authorization.openapi.yaml` around lines 799 - 808, The alg schema for policy.KasPublicKey currently uses a numeric exclusion (not: enum: [- 0]) alongside a $ref to policy.KasPublicKeyAlgEnum (a string enum), which fails to exclude the unspecified string value; locate every occurrence of the alg property referencing policy.KasPublicKeyAlgEnum (e.g., policy.KasPublicKey.alg in authorization.openapi.yaml and the listed policy files) and replace the numeric guard with a string exclusion that matches the enum member (for example change not: enum: [0] to not: const: "KAS_PUBLIC_KEY_ALG_ENUM_UNSPECIFIED" or not: enum: ["KAS_PUBLIC_KEY_ALG_ENUM_UNSPECIFIED"]); ensure the $ref to policy.KasPublicKeyAlgEnum remains and apply the same replacement in all files mentioned.docs/openapi/policy/registeredresources/registered_resources.openapi.yaml (1)
1380-1425:⚠️ Potential issue | 🟠 MajorFix parent
additionalProperties: falsesooneOf/allOfbranch fields aren’t rejected as “additional”.
policy.registeredresources.ActionAttributeValue(1387-1432): the parent object setsadditionalProperties: false, butactionId/actionNameandattributeValueFqn/attributeValueIdare only defined insideoneOfbranches—so validators reject the intended fields.policy.registeredresources.GetRegisteredResourceRequest(1569-1605): same pattern—additionalProperties: falseat the parent allows onlynamespaceFqn/namespaceId, whileid/nameexist only inoneOfbranches and are rejected.Promote these fields into the parent schema’s
properties(keeping therequiredlists inside the branch schemas), or removeadditionalProperties: falsefrom the parent object. json-schema.org🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/openapi/policy/registeredresources/registered_resources.openapi.yaml` around lines 1380 - 1425, The parent schemas policy.registeredresources.ActionAttributeValue and policy.registeredresources.GetRegisteredResourceRequest set additionalProperties: false while defining actionable fields only inside oneOf/allOf branches, causing validators to treat actionId/actionName and attributeValueFqn/attributeValueId (and id/name for GetRegisteredResourceRequest) as additional; fix by promoting those branch properties into the parent schema's properties (declare actionId, actionName, attributeValueFqn, attributeValueId in ActionAttributeValue and id/name/namespaceFqn/namespaceId in GetRegisteredResourceRequest) and keep the branch-specific required arrays inside the oneOf branches, or alternatively remove additionalProperties: false from the parent—ensure the parent titles (ActionAttributeValue, GetRegisteredResourceRequest) reflect the updated properties so branch validation succeeds.docs/openapi/policy/obligations/obligations.openapi.yaml (1)
981-990:⚠️ Potential issue | 🟠 MajorFix
KasPublicKey.algnot.enummismatch (0 vsKasPublicKeyAlgEnumstrings)In
docs/openapi/policy/obligations/obligations.openapi.yaml,docs/openapi/policy/registeredresources/registered_resources.openapi.yaml, anddocs/openapi/policy/resourcemapping/resource_mapping.openapi.yaml,policy.KasPublicKey.algusesnot: { enum: [0] }, but#/components/schemas/policy.KasPublicKeyAlgEnumistype: stringwith enum values includingKAS_PUBLIC_KEY_ALG_ENUM_UNSPECIFIED. Since0won’t match any of the allowed string enum members, thenotclause doesn’t exclude the intended UNSPECIFIED value. Replace0withKAS_PUBLIC_KEY_ALG_ENUM_UNSPECIFIEDin thenot.enumlist. (json-schema.org)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/openapi/policy/obligations/obligations.openapi.yaml` around lines 981 - 990, The not.enum value for KasPublicKey.alg currently contains the numeric 0 which doesn't match the string-based policy.KasPublicKeyAlgEnum; update the not.enum list to use the enum member name KAS_PUBLIC_KEY_ALG_ENUM_UNSPECIFIED instead of 0 in the KasPublicKey.alg schema (affecting the KasPublicKey definition in obligations.openapi.yaml, registered_resources.openapi.yaml, and resource_mapping.openapi.yaml) so the not constraint correctly excludes the unspecified string enum value from policy.KasPublicKeyAlgEnum.docs/openapi/policy/subjectmapping/subject_mapping.openapi.yaml (1)
848-857:⚠️ Potential issue | 🟠 MajorFix ineffective
algUNSPECIFIED guard in both OpenAPI filesIn
docs/openapi/policy/subjectmapping/subject_mapping.openapi.yaml(lines 848-857) anddocs/openapi/policy/unsafe/unsafe.openapi.yaml(lines 814-823),alguses$ref: '#/components/schemas/policy.KasPublicKeyAlgEnum'wherepolicy.KasPublicKeyAlgEnumis a string enum includingKAS_PUBLIC_KEY_ALG_ENUM_UNSPECIFIED, but thenotguard excludesenum: [0](a number). Since no validalgstring value equals numeric0, thenotcondition never matches, soKAS_PUBLIC_KEY_ALG_ENUM_UNSPECIFIEDremains permitted by the contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/openapi/policy/subjectmapping/subject_mapping.openapi.yaml` around lines 848 - 857, The `alg` property uses $ref to policy.KasPublicKeyAlgEnum (a string enum) but the `not` guard incorrectly excludes numeric 0 so the UNSPECIFIED string value still passes; update the `not.enum` to exclude the actual enum member string (e.g., "KAS_PUBLIC_KEY_ALG_ENUM_UNSPECIFIED") or otherwise use `not.const: "KAS_PUBLIC_KEY_ALG_ENUM_UNSPECIFIED"` for the `alg` property in both subject_mapping.openapi.yaml and unsafe.openapi.yaml so the UNSPECIFIED string is correctly forbidden; locate the `alg` field and modify its `not` clause referencing policy.KasPublicKeyAlgEnum accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/openapi/authorization/authorization.openapi.yaml`:
- Around line 651-656: The examples for the schema symbol
google.protobuf.Timestamp are invalid for format: date-time (they are duration
literals like "1s"); update the examples to RFC3339 date-time strings (e.g.
"1970-01-01T00:00:01Z" and "2020-01-01T00:00:00.000340012Z" or similar ISO8601
UTC timestamps) so they match the declared format: date-time and the schema
description; ensure this change is applied wherever google.protobuf.Timestamp is
declared.
- Around line 297-359: The parent schema authorization.Entity (and other similar
schemas mentioned) has additionalProperties: false at the top level while all
valid variant keys live inside oneOf branches, causing legitimate payloads to be
rejected; update the schema by removing or relaxing the parent-level
additionalProperties and instead add additionalProperties: false inside each
oneOf branch (or refactor to an allOf + shared base or use unevaluatedProperties
where supported) so that when a branch is selected its properties are allowed;
target the authorization.Entity schema (and the other listed schemas like
entity.Entity, authorization.v2.Resource, policy.Action,
authorization.v2.EntityIdentifier, kas.KeyAccessRewrapResult) and ensure each
oneOf variant object includes its own additionalProperties rule or convert to an
allOf pattern that composes the shared fields with branch-specific constraints.
In `@docs/openapi/policy/attributes/attributes.openapi.yaml`:
- Around line 1097-1100: The OpenAPI schemas emit array-only keywords
(minItems/maxItems/uniqueItems) inside scalar item schemas — locate the array
schemas in attributes.openapi.yaml and objects.openapi.yaml where you have an
enclosing "type: array" with "items: { type: string, minItems: ... }" (look for
the "items" blocks shown in the diff) and remove any
minItems/maxItems/uniqueItems from the scalar "items" objects; then add those
constraints to the enclosing array node (the object with "type: array") so the
constraints apply to the array itself rather than the item schema.
- Around line 1150-1153: The schema for policy.KasPublicKey.alg is attempting to
exclude the UNSPECIFIED enum by using not.enum: [0], but the generated enums are
strings so that exclusion never matches; update the YAMLs (where you see
policy.KasPublicKey.alg and references to policy.KasPublicKeyAlgEnum /
KAS_PUBLIC_KEY_ALG_ENUM_UNSPECIFIED) to exclude the string value instead (change
not.enum: - 0 to not.enum: - "0") so the UNSPECIFIED value is actually rejected
by the OpenAPI schema.
- Around line 1946-1984: The top-level deprecated id property was moved out of
the oneOf, so the oneOf no longer enforces exclusivity; restore exclusivity by
moving the deprecated id back into the oneOf as its own alternative (i.e., add a
oneOf branch with properties: id (type: string, format: uuid, deprecated: true)
and required: [id]) for each affected schema (e.g., GetAttributeRequest with
attributeId/fqn, and the equivalent request schemas in
key_access_server_registry and namespaces that reference
valueId/kasId/namespaceId), so oneOf again enforces mutual exclusion between id
and the new identifiers. Ensure additionalProperties/description stay unchanged.
- Around line 2352-2397: The connect.error schema uses a single "detail"
property of type google.protobuf.Any but Connect's HTTP JSON uses "details" as
an array of google.protobuf.Any; update the connect.error schema (symbol:
connect.error) in the affected OpenAPI files to rename "detail" to "details" and
change its type to an array of google.protobuf.Any (i.e., items referencing
google.protobuf.Any), preserving additionalProperties and descriptions; apply
the same change in the other listed schemas (those that define connect.error) so
the property name and cardinality match Connect’s HTTP error representation.
In `@docs/openapi/policy/kasregistry/key_access_server_registry.openapi.yaml`:
- Around line 1325-1371: The schema GetKeyAccessServerRequest currently declares
"id" as a top-level property so objects like {id, kasId} validate; to enforce
the intended exclusivity, remove the top-level properties.id and instead add a
separate oneOf branch that defines an id-only object (properties: id {type:
string, format: uuid}, required: [id], deprecated: true) alongside the existing
kasId/name/uri branches — i.e. make the top-level oneOf include the id-only
branch and the kasId-only, name-only, uri-only branches so requests must match
exactly one branch.
In `@docs/openapi/policy/obligations/obligations.openapi.yaml`:
- Around line 726-731: The examples for the schema google.protobuf.Timestamp
currently use duration literals; update the examples to valid RFC3339 date-time
strings (e.g., "2023-01-02T15:04:05Z" and "2023-01-02T15:04:05.000340012Z") and
replace the duration examples in the google.protobuf.Timestamp example blocks so
all three files (the google.protobuf.Timestamp schema in
obligations.openapi.yaml, registered_resources.openapi.yaml, and
resource_mapping.openapi.yaml) use the same RFC3339 example values for
consistency.
- Around line 822-857: The policy.Action schema currently defines custom and
standard only inside the oneOf branches while the parent object sets
additionalProperties: false and lacks those properties, causing validators to
reject them; fix by adding top-level properties entries for "custom" and
"standard" in the policy.Action properties block (matching the types/$ref used
in the oneOf branches) and keep the existing oneOf to enforce which variant is
present (or adjust the oneOf to reference the top-level properties), ensuring
additionalProperties: false no longer blocks the defined fields; update the
policy.Action schema (symbols: policy.Action, custom, standard,
additionalProperties, oneOf) accordingly in all three files.
In `@docs/openapi/policy/subjectmapping/subject_mapping.openapi.yaml`:
- Around line 593-598: The examples for the schema named
google.protobuf.Timestamp are invalid duration literals; update the examples in
docs/openapi/policy/subjectmapping/subject_mapping.openapi.yaml (and the same
change in docs/openapi/policy/unsafe/unsafe.openapi.yaml) so they are valid RFC
3339 date-time strings that match the declared type/format (e.g.,
"2023-01-01T00:00:00Z", "2023-01-01T12:34:56.789Z") instead of
"1s"/"1.000340012s", ensuring the google.protobuf.Timestamp entry remains type:
string and format: date-time.
- Around line 689-724: The schemas policy.Action and policy.PublicKey are
unsatisfiable because their variant fields live only inside oneOf branches while
the parent sets additionalProperties: false; fix by either (A) moving the
variant properties (for policy.Action: custom, standard; for policy.PublicKey:
cached, remote) into the parent properties block and keep the oneOf for
required/validation, or (B) replace the parent-level additionalProperties: false
with unevaluatedProperties: false so properties validated by oneOf branches are
not treated as "additional"; update the policy.Action and policy.PublicKey
schema blocks accordingly and retain existing oneOf titles/required constraints.
- Around line 1821-1866: The connect.error schema currently defines a singular
property named "detail" of type google.protobuf.Any but must match
ConnectRPC/google.rpc.Status which uses a repeated "details" field; update the
connect.error schema (the connect.error component) to replace the "detail"
property with "details" as an array of google.protobuf.Any (e.g., type: array,
items: $ref: '`#/components/schemas/google.protobuf.Any`'), keep
additionalProperties/title/description unchanged and ensure any references or
examples that mention google.rpc.Status.details remain consistent with the new
plural field name.
In `@service/policy/selectors_test.go`:
- Around line 12-25: Extend TestSearchTermValidation to cover edge cases: add
assertions that an empty Term and a whitespace-only Term pass validation, that
multibyte/unicode strings are validated according to the intended length
semantics (add a test using runes vs bytes to confirm whether policy.Search.Term
limit applies to characters or bytes), and that strings containing special
characters pass/fail appropriately; update tests that call protovalidate.New()
and v.Validate(&policy.Search{Term: ...}) to include these new cases and assert
success or string.max_len error as appropriate.
In `@service/policy/selectors.proto`:
- Around line 67-69: Add a short comment above the Search.message or the term
field explaining why the string max_len is set to 253 (e.g., to match DNS FQDN
limits used by the system and to constrain substring matching semantics in
Postgres), so future maintainers see the rationale; reference the Search message
and the term field with the (buf.validate.field).string.max_len = 253 constraint
in the comment.
---
Outside diff comments:
In `@docs/openapi/authorization/authorization.openapi.yaml`:
- Around line 799-808: The alg schema for policy.KasPublicKey currently uses a
numeric exclusion (not: enum: [- 0]) alongside a $ref to
policy.KasPublicKeyAlgEnum (a string enum), which fails to exclude the
unspecified string value; locate every occurrence of the alg property
referencing policy.KasPublicKeyAlgEnum (e.g., policy.KasPublicKey.alg in
authorization.openapi.yaml and the listed policy files) and replace the numeric
guard with a string exclusion that matches the enum member (for example change
not: enum: [0] to not: const: "KAS_PUBLIC_KEY_ALG_ENUM_UNSPECIFIED" or not:
enum: ["KAS_PUBLIC_KEY_ALG_ENUM_UNSPECIFIED"]); ensure the $ref to
policy.KasPublicKeyAlgEnum remains and apply the same replacement in all files
mentioned.
In `@docs/openapi/common/common.openapi.yaml`:
- Around line 23-56: The shared schemas common.IdFqnIdentifier and
common.IdNameIdentifier currently allow an empty object because the conditional
required branches were removed; restore the "at least one identifier" constraint
by adding a oneOf to each schema that enforces either id is present or the other
identifier is present (for common.IdFqnIdentifier oneOf: required ['id'] OR
required ['fqn']; for common.IdNameIdentifier oneOf: required ['id'] OR required
['name']), keeping additionalProperties: false and existing property definitions
intact so consumers cannot pass {}.
In `@docs/openapi/policy/obligations/obligations.openapi.yaml`:
- Around line 981-990: The not.enum value for KasPublicKey.alg currently
contains the numeric 0 which doesn't match the string-based
policy.KasPublicKeyAlgEnum; update the not.enum list to use the enum member name
KAS_PUBLIC_KEY_ALG_ENUM_UNSPECIFIED instead of 0 in the KasPublicKey.alg schema
(affecting the KasPublicKey definition in obligations.openapi.yaml,
registered_resources.openapi.yaml, and resource_mapping.openapi.yaml) so the not
constraint correctly excludes the unspecified string enum value from
policy.KasPublicKeyAlgEnum.
In `@docs/openapi/policy/registeredresources/registered_resources.openapi.yaml`:
- Around line 1380-1425: The parent schemas
policy.registeredresources.ActionAttributeValue and
policy.registeredresources.GetRegisteredResourceRequest set
additionalProperties: false while defining actionable fields only inside
oneOf/allOf branches, causing validators to treat actionId/actionName and
attributeValueFqn/attributeValueId (and id/name for
GetRegisteredResourceRequest) as additional; fix by promoting those branch
properties into the parent schema's properties (declare actionId, actionName,
attributeValueFqn, attributeValueId in ActionAttributeValue and
id/name/namespaceFqn/namespaceId in GetRegisteredResourceRequest) and keep the
branch-specific required arrays inside the oneOf branches, or alternatively
remove additionalProperties: false from the parent—ensure the parent titles
(ActionAttributeValue, GetRegisteredResourceRequest) reflect the updated
properties so branch validation succeeds.
In `@docs/openapi/policy/subjectmapping/subject_mapping.openapi.yaml`:
- Around line 848-857: The `alg` property uses $ref to
policy.KasPublicKeyAlgEnum (a string enum) but the `not` guard incorrectly
excludes numeric 0 so the UNSPECIFIED string value still passes; update the
`not.enum` to exclude the actual enum member string (e.g.,
"KAS_PUBLIC_KEY_ALG_ENUM_UNSPECIFIED") or otherwise use `not.const:
"KAS_PUBLIC_KEY_ALG_ENUM_UNSPECIFIED"` for the `alg` property in both
subject_mapping.openapi.yaml and unsafe.openapi.yaml so the UNSPECIFIED string
is correctly forbidden; locate the `alg` field and modify its `not` clause
referencing policy.KasPublicKeyAlgEnum accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2a2a25cd-745b-4647-810e-d9bcd3956a99
⛔ Files ignored due to path filters (7)
protocol/go/policy/attributes/attributes.pb.gois excluded by!**/*.pb.goprotocol/go/policy/kasregistry/key_access_server_registry.pb.gois excluded by!**/*.pb.goprotocol/go/policy/namespaces/namespaces.pb.gois excluded by!**/*.pb.goprotocol/go/policy/obligations/obligations.pb.gois excluded by!**/*.pb.goprotocol/go/policy/registeredresources/registered_resources.pb.gois excluded by!**/*.pb.goprotocol/go/policy/selectors.pb.gois excluded by!**/*.pb.goprotocol/go/policy/subjectmapping/subject_mapping.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (29)
docs/grpc/index.htmldocs/openapi/authorization/authorization.openapi.yamldocs/openapi/authorization/v2/authorization.openapi.yamldocs/openapi/common/common.openapi.yamldocs/openapi/entity/entity.openapi.yamldocs/openapi/entityresolution/entity_resolution.openapi.yamldocs/openapi/entityresolution/v2/entity_resolution.openapi.yamldocs/openapi/kas/kas.openapi.yamldocs/openapi/policy/actions/actions.openapi.yamldocs/openapi/policy/attributes/attributes.openapi.yamldocs/openapi/policy/kasregistry/key_access_server_registry.openapi.yamldocs/openapi/policy/keymanagement/key_management.openapi.yamldocs/openapi/policy/namespaces/namespaces.openapi.yamldocs/openapi/policy/objects.openapi.yamldocs/openapi/policy/obligations/obligations.openapi.yamldocs/openapi/policy/registeredresources/registered_resources.openapi.yamldocs/openapi/policy/resourcemapping/resource_mapping.openapi.yamldocs/openapi/policy/selectors.openapi.yamldocs/openapi/policy/subjectmapping/subject_mapping.openapi.yamldocs/openapi/policy/unsafe/unsafe.openapi.yamldocs/openapi/wellknownconfiguration/wellknown_configuration.openapi.yamlservice/policy/attributes/attributes.protoservice/policy/kasregistry/key_access_server_registry.protoservice/policy/namespaces/namespaces.protoservice/policy/obligations/obligations.protoservice/policy/registeredresources/registered_resources.protoservice/policy/selectors.protoservice/policy/selectors_test.goservice/policy/subjectmapping/subject_mapping.proto
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
🛑 Comments failed to post (14)
docs/openapi/authorization/authorization.openapi.yaml (2)
297-359:
⚠️ Potential issue | 🟠 Major🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail files=( "docs/openapi/authorization/authorization.openapi.yaml" "docs/openapi/authorization/v2/authorization.openapi.yaml" "docs/openapi/entity/entity.openapi.yaml" "docs/openapi/entityresolution/entity_resolution.openapi.yaml" "docs/openapi/entityresolution/v2/entity_resolution.openapi.yaml" "docs/openapi/kas/kas.openapi.yaml" "docs/openapi/policy/actions/actions.openapi.yaml" ) for f in "${files[@]}"; do echo "===== $f =====" if [[ ! -f "$f" ]]; then echo "MISSING FILE" continue fi # Show all oneOf blocks that are near additionalProperties:false rg -n --context=6 "oneOf:" "$f" | head -n 80 echo "--- occurrences of additionalProperties: false ---" rg -n "additionalProperties:\s*false" "$f" || true echo "--- show surrounding lines for each additionalProperties:false ---" while IFS=: read -r lineNo _; do start=$((lineNo-20)) end=$((lineNo+30)) echo "---- context around $lineNo ----" sed -n "${start},${end}p" "$f" | nl -ba | sed 's/^/CTX: /' done < <(rg -n "additionalProperties:\s*false" "$f" | head -n 50) doneRepository: opentdf/platform
Length of output: 2772
🏁 Script executed:
#!/bin/bash set -euo pipefail files=( "docs/openapi/authorization/authorization.openapi.yaml" "docs/openapi/authorization/v2/authorization.openapi.yaml" "docs/openapi/entity/entity.openapi.yaml" "docs/openapi/entityresolution/entity_resolution.openapi.yaml" "docs/openapi/entityresolution/v2/entity_resolution.openapi.yaml" "docs/openapi/kas/kas.openapi.yaml" "docs/openapi/policy/actions/actions.openapi.yaml" ) python3 - <<'PY' from pathlib import Path files = [ "docs/openapi/authorization/authorization.openapi.yaml", "docs/openapi/authorization/v2/authorization.openapi.yaml", "docs/openapi/entity/entity.openapi.yaml", "docs/openapi/entityresolution/entity_resolution.openapi.yaml", "docs/openapi/entityresolution/v2/entity_resolution.openapi.yaml", "docs/openapi/kas/kas.openapi.yaml", "docs/openapi/policy/actions/actions.openapi.yaml", ] for f in files: p = Path(f) if not p.exists(): print(f"===== MISSING: {f} =====") continue lines = p.read_text(encoding="utf-8").splitlines() oneofs = [] adds = [] for i, line in enumerate(lines): if "oneOf:" in line: indent = len(line) - len(line.lstrip(" ")) oneofs.append((i, indent, line.strip())) if "additionalProperties:" in line and "false" in line: indent = len(line) - len(line.lstrip(" ")) adds.append((i, indent, line.strip())) # Find additionalProperties false that is "inside" an object having a same-indent oneOf shortly above it. hits = [] for ai, aindent, astrip in adds: for oi, oindent, ostrip in oneofs: if oindent == aindent and 0 < ai - oi <= 120: # likely in same object scope; record the closest oneOf above hits.append((ai, aindent, oi)) break print(f"\n===== {f} =====") if not hits: print("No same-indent oneOf/additionalProperties:false pairs found in proximity.") continue # de-dupe by additionalProperties line index seen=set() unique=[] for h in hits: if h[0] not in seen: unique.append(h); seen.add(h[0]) unique.sort() for ai, aindent, oi in unique: print(f"\n-- oneOf at line {oi+1} (indent {aindent}) and additionalProperties:false at line {ai+1} --") start = max(0, oi-5) end = min(len(lines), ai+20) for j in range(start, end): # keep output readable print(f"{j+1:4d}:{lines[j]}") PYRepository: opentdf/platform
Length of output: 50375
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' import re from pathlib import Path files = [ "docs/openapi/authorization/authorization.openapi.yaml", "docs/openapi/authorization/v2/authorization.openapi.yaml", "docs/openapi/entity/entity.openapi.yaml", "docs/openapi/entityresolution/entity_resolution.openapi.yaml", "docs/openapi/entityresolution/v2/entity_resolution.openapi.yaml", "docs/openapi/kas/kas.openapi.yaml", "docs/openapi/policy/actions/actions.openapi.yaml", ] def indent_of(line): return len(line) - len(line.lstrip(" ")) def find_pairs(lines): pairs=[] oneofs=[] adds=[] for i,l in enumerate(lines): if re.match(r'^\s*oneOf:\s*$', l): oneofs.append((i, indent_of(l))) if re.match(r'^\s*additionalProperties:\s*false\s*$', l): adds.append((i, indent_of(l))) # candidate: same-indent within next 200 lines for oi, ind in oneofs: cand=None for ai, aind in adds: if aind==ind and 0 < ai-oi <= 200: cand=ai break if cand is not None: pairs.append((oi, cand, ind)) return pairs def extract_schema_title(lines, start, ind): # search backwards from 'start' for 'title:' at indent ind+2 or ind+0 (best-effort) for j in range(start, max(-1,start-80), -1): l=lines[j] if re.match(r'^\s*title:\s*.+', l): if indent_of(l) in (ind+2, ind): return l.strip() return None def extract_parent_properties_keys(lines, oneof_i, ind, add_i): # parent properties at indent == ind and after oneOf line (best-effort: first "properties:" at indent ind) for j in range(oneof_i, add_i): l=lines[j] if re.match(r'^\s*properties:\s*$', l) and indent_of(l)==ind: # collect subsequent property keys at indent ind+2 until next sibling key (title/description/additionalProperties/oneOf/etc) keys=[] for k in range(j+1, add_i): lk=lines[k] if indent_of(lk)<=ind: break m=re.match(r'^(\s*)([A-Za-z0-9_]+):\s*$', lk) if m and len(m.group(1))==ind+2: keys.append(m.group(2)) return keys return [] def extract_branch_keys(lines, oneof_i, add_i, ind): # within oneOf range: capture keys declared under branch "- properties:" blocks keys=set() # brute regex by indent: keys lines at indent ind+4 (as in example) key_indent=ind+4 for j in range(oneof_i, add_i): l=lines[j] # match "- properties:" then subsequent property keys at indent ind+4 if re.match(r'^\s*-\s*properties:\s*$', l): # read forward until indent <= ind+2 for k in range(j+1, min(add_i, j+80)): lk=lines[k] if indent_of(lk)<=ind+2: break m=re.match(r'^(\s*)([A-Za-z0-9_]+):\s*$', lk) if m and len(m.group(1))==key_indent: keys.add(m.group(2)) return sorted(keys) for f in files: p=Path(f) if not p.exists(): print(f"\n===== MISSING {f} =====") continue lines=p.read_text(encoding="utf-8").splitlines() pairs=find_pairs(lines) # only show first 1-2 pairs per file to keep output small, but enough to check claim shown=0 print(f"\n===== {f} =====") if not pairs: print("No same-indent oneOf/additionalProperties:false pairs found within 200 lines.") continue for (oi, ai, ind) in pairs[:3]: title=extract_schema_title(lines, oi, ind) or "(no title found)" parent_keys=extract_parent_properties_keys(lines, oi, ind, ai) branch_keys=extract_branch_keys(lines, oi, ai, ind) overlap=sorted(set(parent_keys) & set(branch_keys)) print(f"- oneOf line {oi+1}, additionalProperties:false line {ai+1}, indent {ind}") print(f" title: {title}") print(f" parent properties keys: {parent_keys}") print(f" oneOf-branch property keys: {branch_keys}") print(f" overlap: {overlap}") shown+=1 if shown>=2: break PYRepository: opentdf/platform
Length of output: 2895
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' import re from pathlib import Path files = [ "docs/openapi/authorization/authorization.openapi.yaml", "docs/openapi/authorization/v2/authorization.openapi.yaml", "docs/openapi/entity/entity.openapi.yaml", "docs/openapi/entityresolution/entity_resolution.openapi.yaml", "docs/openapi/entityresolution/v2/entity_resolution.openapi.yaml", "docs/openapi/kas/kas.openapi.yaml", "docs/openapi/policy/actions/actions.openapi.yaml", ] def indent_of(s): return len(s) - len(s.lstrip(" ")) def find_schema_key(lines, from_i, indent_key): for j in range(from_i, max(-1, from_i-2000), -1): m = re.match(r'^' + ' ' * indent_key + r'([A-Za-z0-9_.-]+):\s*$', lines[j]) if m: return m.group(1) return None def extract_keys_after_properties(lines, props_i, props_indent): keys=[] # collect immediate keys at props_indent+2 key_indent = props_indent + 2 for k in range(props_i+1, min(len(lines), props_i+200)): lk=lines[k] if not lk.strip(): continue if indent_of(lk) <= props_indent: break m = re.match(r'^' + ' ' * key_indent + r'([A-Za-z0-9_.-]+):\s*$', lk) if m: keys.append(m.group(1)) return keys def extract_branch_properties_keys(lines, oneof_i, end_i, oneof_indent): # find "- properties:" lines within oneOf; keys follow at dash_indent+4 keys=set() for j in range(oneof_i, min(len(lines), end_i+1)): m = re.match(r'^' + ' ' * (oneof_indent+2) + r'-\s*properties:\s*$', lines[j]) if not m: continue dash_indent = oneof_indent+2 key_indent = dash_indent + 4 # collect keys until indent <= dash_indent for k in range(j+1, min(len(lines), j+120)): lk=lines[k] if not lk.strip(): continue if indent_of(lk) <= dash_indent: break m2 = re.match(r'^' + ' ' * key_indent + r'([A-Za-z0-9_.-]+):\s*$', lk) if m2: keys.add(m2.group(1)) return sorted(keys) for f in files: p=Path(f) if not p.exists(): print(f"\n===== MISSING: {f} =====") continue lines=p.read_text(encoding="utf-8").splitlines() # Find first additionalProperties:false and nearest preceding oneOf at same indent best=None for ai, l in enumerate(lines): m = re.match(r'^( *)additionalProperties:\s*false\s*$', l) if not m: continue ind = len(m.group(1)) # search backwards for oneOf at same indent within 200 lines oneof_i=None for j in range(ai-1, max(-1, ai-1-200), -1): m2 = re.match(r'^( *)oneOf:\s*$', lines[j]) if m2 and len(m2.group(1))==ind: oneof_i=j break if oneof_i is None: continue best=(oneof_i, ai, ind) break print(f"\n===== {f} =====") if not best: print("No same-indent oneOf/additionalProperties:false pair found (heuristic).") continue oneof_i, add_i, ind = best schema_indent = ind - 2 schema_key = find_schema_key(lines, oneof_i, schema_indent) or "(schema key not found)" parent_props_keys=[] # find first 'properties:' at indent ind after oneOf and before add_i props_i=None for k in range(oneof_i, min(add_i, oneof_i+300)): m3 = re.match(r'^' + ' ' * ind + r'properties:\s*$', lines[k]) if m3: props_i=k break if props_i is not None: parent_props_keys = extract_keys_after_properties(lines, props_i, ind) branch_keys = extract_branch_properties_keys(lines, oneof_i, add_i, ind) print(f"schema: {schema_key}") print(f"oneOf: line {oneof_i+1} (indent {ind}), additionalProperties:false: line {add_i+1}") print(f"parent properties keys (at indent {ind+2}): {parent_props_keys}") print(f"oneOf-branch properties keys (immediate keys under branch properties): {branch_keys}") if parent_props_keys: missing=[k for k in branch_keys if k not in parent_props_keys] print(f"branch keys missing from parent properties (would be 'additional'): {missing[:30]}{'...' if len(missing)>30 else ''}") else: print("parent properties keys not found by heuristic") PYRepository: opentdf/platform
Length of output: 3170
🏁 Script executed:
#!/bin/bash set -euo pipefail declare -A ranges ranges["docs/openapi/authorization/authorization.openapi.yaml"]="290 370" ranges["docs/openapi/authorization/v2/authorization.openapi.yaml"]="235 320" ranges["docs/openapi/entity/entity.openapi.yaml"]="1 80" ranges["docs/openapi/entityresolution/entity_resolution.openapi.yaml"]="85 175" ranges["docs/openapi/entityresolution/v2/entity_resolution.openapi.yaml"]="80 150" ranges["docs/openapi/kas/kas.openapi.yaml"]="165 215" ranges["docs/openapi/policy/actions/actions.openapi.yaml"]="405 480" for f in "${!ranges[@]}"; do read -r start end <<< "${ranges[$f]}" echo "===== $f ($start-$end) =====" if [[ ! -f "$f" ]]; then echo "MISSING"; continue; fi sed -n "${start},${end}p" "$f" | nl -ba | sed 's/^/LN: /' echo doneRepository: opentdf/platform
Length of output: 193
🏁 Script executed:
#!/bin/bash set -euo pipefail declare -A ranges ranges["docs/openapi/authorization/authorization.openapi.yaml"]="290 370" ranges["docs/openapi/authorization/v2/authorization.openapi.yaml"]="235 320" ranges["docs/openapi/entity/entity.openapi.yaml"]="1 120" ranges["docs/openapi/entityresolution/entity_resolution.openapi.yaml"]="85 190" ranges["docs/openapi/entityresolution/v2/entity_resolution.openapi.yaml"]="80 155" ranges["docs/openapi/kas/kas.openapi.yaml"]="165 230" ranges["docs/openapi/policy/actions/actions.openapi.yaml"]="405 485" for f in "${!ranges[@]}"; do read -r start end <<< "${ranges[$f]}" echo "===== $f ($start-$end) =====" if [[ ! -f "$f" ]]; then echo "MISSING"; echo; continue; fi awk -v s="$start" -v e="$end" '{ln=NR; if(ln>=s && ln<=e) printf "LN:%5d %s\n", ln, $0}' "$f" echo doneRepository: opentdf/platform
Length of output: 25016
🏁 Script executed:
#!/bin/bash set -euo pipefail f="docs/openapi/kas/kas.openapi.yaml" python3 - <<'PY' import re from pathlib import Path p=Path("docs/openapi/kas/kas.openapi.yaml") lines=p.read_text(encoding="utf-8").splitlines() def indent_of(line): return len(line)-len(line.lstrip(" ")) oneofs=[] adds=[] for i,l in enumerate(lines): if re.match(r'^\s*oneOf:\s*$', l): oneofs.append((i, indent_of(l))) if re.match(r'^\s*additionalProperties:\s*false\s*$', l): adds.append((i, indent_of(l))) # find pairs where same indentation and additionalProperties:false occurs after oneOf within 120 lines pairs=[] for oi,oi_indent in oneofs: for ai,ai_indent in adds: if oi_indent==ai_indent and 0 < ai-oi <= 120: pairs.append((oi,ai,oi_indent)) # print a small snippet around each pair print(f"Found {len(pairs)} oneOf/additionalProperties:false same-indent pairs (within 120 lines).") for idx,(oi,ai,ind) in enumerate(pairs[:20],1): # find schema title just above the oneOf if possible title=None for j in range(oi, max(-1,oi-200), -1): m=re.match(r'^' + ' '*ind + r'title:\s*(.+)\s*$', lines[j]) if m: title=m.group(1) break print(f"\n[{idx}] oneOf at line {oi+1} indent {ind}, additionalProperties:false at line {ai+1} indent {ind}, title={title}") start=max(0,oi-10); end=min(len(lines),ai+25) for k in range(start,end): print(f"{k+1:5d}:{lines[k]}") PYRepository: opentdf/platform
Length of output: 36769
Fix parent-level
additionalProperties: falsewithoneOf-only variant fields (breaks validation/codegen)In these OpenAPI schemas, the parent object keeps
additionalProperties: falsewhile the allowed variant-only keys live solely insideoneOfbranches (e.g.,authorization.Entity,entity.Entity,authorization.Entity(entity resolution),authorization.v2.Resource,policy.Action,authorization.v2.EntityIdentifier, andkas.KeyAccessRewrapResult). With JSON Schema semantics, those variant keys are treated as “additional” by the parent-level constraint, so legitimate payloads are rejected.Move
additionalProperties: falseinto eachoneOfbranch (or refactor using anallOfpattern / unevaluated-properties approach) so variant-only properties are permitted when their branch is selected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/openapi/authorization/authorization.openapi.yaml` around lines 297 - 359, The parent schema authorization.Entity (and other similar schemas mentioned) has additionalProperties: false at the top level while all valid variant keys live inside oneOf branches, causing legitimate payloads to be rejected; update the schema by removing or relaxing the parent-level additionalProperties and instead add additionalProperties: false inside each oneOf branch (or refactor to an allOf + shared base or use unevaluatedProperties where supported) so that when a branch is selected its properties are allowed; target the authorization.Entity schema (and the other listed schemas like entity.Entity, authorization.v2.Resource, policy.Action, authorization.v2.EntityIdentifier, kas.KeyAccessRewrapResult) and ensure each oneOf variant object includes its own additionalProperties rule or convert to an allOf pattern that composes the shared fields with branch-specific constraints.
651-656:
⚠️ Potential issue | 🟡 Minor🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail files=( "docs/openapi/authorization/authorization.openapi.yaml" "docs/openapi/authorization/v2/authorization.openapi.yaml" "docs/openapi/common/common.openapi.yaml" "docs/openapi/policy/actions/actions.openapi.yaml" ) for f in "${files[@]}"; do echo "=== $f ===" if [[ ! -f "$f" ]]; then echo "MISSING" continue fi # Print the google.protobuf.Timestamp block with nearby lines (keep output small) rg -n --no-heading --context 3 'google\.protobuf\.Timestamp:' "$f" || true # Print the examples/format lines nearby for quick inspection rg -n --no-heading --context 2 'google\.protobuf\.Timestamp:' "$f" | head -n 50 || true rg -n --no-heading 'examples:|format:|RFC3339|date-time' "$f" | head -n 80 || true echo doneRepository: opentdf/platform
Length of output: 2810
🏁 Script executed:
#!/bin/bash set -euo pipefail ranges=( "docs/openapi/authorization/authorization.openapi.yaml:645-660" "docs/openapi/authorization/v2/authorization.openapi.yaml:680-700" "docs/openapi/common/common.openapi.yaml:105-130" "docs/openapi/policy/actions/actions.openapi.yaml:310-340" ) for spec in "${ranges[@]}"; do file="${spec%%:*}" range="${spec#*:}" echo "=== $file ($range) ===" sed -n "${range}p" "$file" | nl -ba -w2 -s': ' | sed "s/^/${file} /" echo done # Also check whether any RFC3339 description exists near those blocks for file in \ "docs/openapi/authorization/authorization.openapi.yaml" \ "docs/openapi/authorization/v2/authorization.openapi.yaml" \ "docs/openapi/common/common.openapi.yaml" \ "docs/openapi/policy/actions/actions.openapi.yaml"; do echo "=== $file: RFC3339 near Timestamp ===" # small window around first match for Timestamp line=$(rg -n 'google\.protobuf\.Timestamp:' "$file" | head -n1 | cut -d: -f1) if [[ -n "${line:-}" ]]; then start=$((line-20)) end=$((line+40)) sed -n "${start},${end}p" "$file" | nl -ba -w2 -s': ' | head -n 120 fi echo doneRepository: opentdf/platform
Length of output: 298
🏁 Script executed:
#!/bin/bash set -euo pipefail print_range() { local file="$1" local start="$2" local end="$3" echo "=== $file (${start}-${end}) ===" awk -v s="$start" -v e="$end" 'NR>=s && NR<=e {printf "%d: %s\n", NR, $0}' "$file" echo } print_range "docs/openapi/authorization/authorization.openapi.yaml" 645 660 print_range "docs/openapi/authorization/v2/authorization.openapi.yaml" 680 700 print_range "docs/openapi/common/common.openapi.yaml" 105 130 print_range "docs/openapi/policy/actions/actions.openapi.yaml" 310 340 for file in \ "docs/openapi/authorization/authorization.openapi.yaml" \ "docs/openapi/authorization/v2/authorization.openapi.yaml" \ "docs/openapi/common/common.openapi.yaml" \ "docs/openapi/policy/actions/actions.openapi.yaml"; do echo "=== $file: nearest `google.protobuf.Timestamp:` block (line +/- 35) ===" line=$(rg -n 'google\.protobuf\.Timestamp:' "$file" | head -n1 | cut -d: -f1) if [[ -z "${line:-}" ]]; then echo "(no Timestamp block found)" continue fi start=$((line-35)) end=$((line+35)) print_range "$file" "$start" "$end" done # Check if any of these blocks contain `format: date-time` and `examples:` together for file in \ "docs/openapi/authorization/authorization.openapi.yaml" \ "docs/openapi/authorization/v2/authorization.openapi.yaml" \ "docs/openapi/common/common.openapi.yaml" \ "docs/openapi/policy/actions/actions.openapi.yaml"; do echo "=== $file: context around `examples:` and `format: date-time` ===" rg -n --no-heading -C 5 'examples:|format: date-time' "$file" | head -n 200 || true echo doneRepository: opentdf/platform
Length of output: 20713
Fix
google.protobuf.Timestampexamples mismatchformat: date-timeIn these specs,
google.protobuf.Timestampis declared withformat: date-time, but theexamplesare duration literals (1s,1.000340012s) while the schema description states it converts to RFC3339 date strings, making the example values misleading/invalid:
docs/openapi/authorization/authorization.openapi.yaml(651-656)docs/openapi/authorization/v2/authorization.openapi.yaml(688-693)docs/openapi/common/common.openapi.yaml(112-117)docs/openapi/policy/actions/actions.openapi.yaml(321-326)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/openapi/authorization/authorization.openapi.yaml` around lines 651 - 656, The examples for the schema symbol google.protobuf.Timestamp are invalid for format: date-time (they are duration literals like "1s"); update the examples to RFC3339 date-time strings (e.g. "1970-01-01T00:00:01Z" and "2020-01-01T00:00:00.000340012Z" or similar ISO8601 UTC timestamps) so they match the declared format: date-time and the schema description; ensure this change is applied wherever google.protobuf.Timestamp is declared.docs/openapi/policy/attributes/attributes.openapi.yaml (4)
1097-1100:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winArray-only keywords were emitted on scalar item schemas in
docs/openapi/policy/attributes/attributes.openapi.yamlanddocs/openapi/policy/objects.openapi.yaml.minItems,maxItems, anduniqueItemsonly apply to arrays, so placing them underitems: { type: string }makes them ineffective noise for validators and generators. Keep those constraints on the enclosing array nodes only.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/openapi/policy/attributes/attributes.openapi.yaml` around lines 1097 - 1100, The OpenAPI schemas emit array-only keywords (minItems/maxItems/uniqueItems) inside scalar item schemas — locate the array schemas in attributes.openapi.yaml and objects.openapi.yaml where you have an enclosing "type: array" with "items: { type: string, minItems: ... }" (look for the "items" blocks shown in the diff) and remove any minItems/maxItems/uniqueItems from the scalar "items" objects; then add those constraints to the enclosing array node (the object with "type: array") so the constraints apply to the array itself rather than the item schema.
1150-1153:
⚠️ Potential issue | 🟠 Major | ⚡ Quick win
policy.KasPublicKey.alglost its UNSPECIFIED exclusion indocs/openapi/policy/attributes/attributes.openapi.yaml,docs/openapi/policy/kasregistry/key_access_server_registry.openapi.yaml,docs/openapi/policy/namespaces/namespaces.openapi.yaml, anddocs/openapi/policy/objects.openapi.yaml. Each schema emitspolicy.KasPublicKeyAlgEnumas strings, sonot.enum: [0]never matches and no longer excludesKAS_PUBLIC_KEY_ALG_ENUM_UNSPECIFIED. This weakens the published contract across every touched policy/KAS surface.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/openapi/policy/attributes/attributes.openapi.yaml` around lines 1150 - 1153, The schema for policy.KasPublicKey.alg is attempting to exclude the UNSPECIFIED enum by using not.enum: [0], but the generated enums are strings so that exclusion never matches; update the YAMLs (where you see policy.KasPublicKey.alg and references to policy.KasPublicKeyAlgEnum / KAS_PUBLIC_KEY_ALG_ENUM_UNSPECIFIED) to exclude the string value instead (change not.enum: - 0 to not.enum: - "0") so the UNSPECIFIED value is actually rejected by the OpenAPI schema.
1946-1984:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winThe identifier
oneOfrefactors no longer enforce deprecated-idexclusivity indocs/openapi/policy/attributes/attributes.openapi.yaml,docs/openapi/policy/kasregistry/key_access_server_registry.openapi.yaml, anddocs/openapi/policy/namespaces/namespaces.openapi.yaml. In each file, the replacement identifiers stay inside theoneOf, but deprecatedidmoved to a sibling top-level property. That means requests like{id, attributeId},{id, valueId},{id, kasId}, or{id, namespaceId}still validate even though the descriptions say those combinations are invalid.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/openapi/policy/attributes/attributes.openapi.yaml` around lines 1946 - 1984, The top-level deprecated id property was moved out of the oneOf, so the oneOf no longer enforces exclusivity; restore exclusivity by moving the deprecated id back into the oneOf as its own alternative (i.e., add a oneOf branch with properties: id (type: string, format: uuid, deprecated: true) and required: [id]) for each affected schema (e.g., GetAttributeRequest with attributeId/fqn, and the equivalent request schemas in key_access_server_registry and namespaces that reference valueId/kasId/namespaceId), so oneOf again enforces mutual exclusion between id and the new identifiers. Ensure additionalProperties/description stay unchanged.
2352-2397:
⚠️ Potential issue | 🟠 Major🧩 Analysis chain
🌐 Web query:
According to Connect RPC's HTTP JSON error representation, is the error details field nameddetails, and can a single error include multiple detail entries?💡 Result:
Yes. In Connect RPC’s HTTP JSON error representation, the error details field is named
details, and it can contain multiple detail entries for a single error. In the Connect error docs, the HTTP JSON example includes a top-leveldetailsfield whose value is a JSON array (e.g.,"details": [ ... ]) [1]. The docs also describe passingdetailsas a list/array of strongly-typed error detail messages [1][2], and the Connect error model includes an optional collection of arbitrary protobuf messages calleddetails[3].Citations:
- 1: https://connectrpc.com/docs/python/errors
- 2: https://connectrpc.com/docs/dart/errors
- 3: https://github.com/connectrpc/connect-go/blob/main/error.go
🏁 Script executed:
#!/bin/bash set -euo pipefail files=( docs/openapi/policy/attributes/attributes.openapi.yaml docs/openapi/policy/kasregistry/key_access_server_registry.openapi.yaml docs/openapi/policy/keymanagement/key_management.openapi.yaml docs/openapi/policy/namespaces/namespaces.openapi.yaml ) for f in "${files[@]}"; do echo "===== $f =====" # Print the connect.error schema block near the mentioned line ranges (best-effort). # Use rg to find the connect.error block header, then show surrounding lines. rg -n --context 5 'connect\.error:' "$f" || true rg -n --context 40 'connect\.error:' "$f" || true # Also search for the singular/plural field usage around the schema. rg -n --context 5 '\bdetail(s)?\b' "$f" | head -n 80 || true # Show any mention of google.rpc.Status.details near connect.error schema. rg -n --context 3 'google\.rpc\.Status\.details' "$f" || true doneRepository: opentdf/platform
Length of output: 19176
Fix
connect.errorschema to match Connect HTTP JSON: it usesdetail(singlegoogle.protobuf.Any) but Connect modelsdetailsas a list/array (multiple entries). This mismatch appears indocs/openapi/policy/attributes/attributes.openapi.yaml(lines 2359-2389),docs/openapi/policy/kasregistry/key_access_server_registry.openapi.yaml(lines 2272-2302),docs/openapi/policy/keymanagement/key_management.openapi.yaml(lines 574-604), anddocs/openapi/policy/namespaces/namespaces.openapi.yaml(lines 1087-1117). Verify the property name and cardinality against Connect’s HTTP error representation before publishing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/openapi/policy/attributes/attributes.openapi.yaml` around lines 2352 - 2397, The connect.error schema uses a single "detail" property of type google.protobuf.Any but Connect's HTTP JSON uses "details" as an array of google.protobuf.Any; update the connect.error schema (symbol: connect.error) in the affected OpenAPI files to rename "detail" to "details" and change its type to an array of google.protobuf.Any (i.e., items referencing google.protobuf.Any), preserving additionalProperties and descriptions; apply the same change in the other listed schemas (those that define connect.error) so the property name and cardinality match Connect’s HTTP error representation.docs/openapi/policy/kasregistry/key_access_server_registry.openapi.yaml (1)
1325-1371:
⚠️ Potential issue | 🟠 Major | ⚡ Quick win
idis no longer actually exclusive with the new identifier fields.With
iddeclared outside theoneOf, requests like{id, kasId}or{id, uri}still satisfy exactly one branch and validate. That no longer matches the description’s “either deprecatedidor one of the new identifiers” contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/openapi/policy/kasregistry/key_access_server_registry.openapi.yaml` around lines 1325 - 1371, The schema GetKeyAccessServerRequest currently declares "id" as a top-level property so objects like {id, kasId} validate; to enforce the intended exclusivity, remove the top-level properties.id and instead add a separate oneOf branch that defines an id-only object (properties: id {type: string, format: uuid}, required: [id], deprecated: true) alongside the existing kasId/name/uri branches — i.e. make the top-level oneOf include the id-only branch and the kasId-only, name-only, uri-only branches so requests must match exactly one branch.docs/openapi/policy/obligations/obligations.openapi.yaml (2)
726-731:
⚠️ Potential issue | 🟠 Major🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail files=( "docs/openapi/policy/obligations/obligations.openapi.yaml" "docs/openapi/policy/registeredresources/registered_resources.openapi.yaml" "docs/openapi/policy/resourcemapping/resource_mapping.openapi.yaml" ) for f in "${files[@]}"; do echo "=== $f (requested lines) ===" if [ -f "$f" ]; then nl -ba "$f" | sed -n '700,760p' | sed -n '1,120p' else echo "Missing file: $f" fi echo done echo "=== rg: google.protobuf.Timestamp blocks ===" rg -n "google\.protobuf\.Timestamp" docs/openapi/policy -S echo echo "=== rg: duration-like examples for Timestamp ===" rg -n "1s|1\.000340012s|format:\s*date-time" docs/openapi/policy/ -SRepository: opentdf/platform
Length of output: 205
🏁 Script executed:
#!/bin/bash set -euo pipefail show_range() { local f="$1" start="$2" end="$3" echo "=== $f (lines $start-$end) ===" if [ -f "$f" ]; then sed -n "${start},${end}p" "$f" else echo "Missing file: $f" fi echo } show_range "docs/openapi/policy/obligations/obligations.openapi.yaml" 716 740 show_range "docs/openapi/policy/registeredresources/registered_resources.openapi.yaml" 540 570 show_range "docs/openapi/policy/resourcemapping/resource_mapping.openapi.yaml" 520 560 echo "=== rg: google.protobuf.Timestamp blocks ===" rg -n "google\.protobuf\.Timestamp" docs/openapi/policy -S || true echo echo "=== rg: duration-like examples for Timestamp ===" rg -n "examples:[[:space:]]*\n[[:space:]]*-[[:space:]]*1s\b|1\.000340012s\b|format:[[:space:]]*date-time\b" docs/openapi/policy -S || trueRepository: opentdf/platform
Length of output: 8463
Replace
google.protobuf.Timestampexamples with RFC3339 timestamps
google.protobuf.Timestampis declared withformat: date-time, but the sharedexamplesuse duration literals (1s,1.000340012s) in:
docs/openapi/policy/obligations/obligations.openapi.yaml(lines 726-731)docs/openapi/policy/registeredresources/registered_resources.openapi.yaml(lines 551-556)docs/openapi/policy/resourcemapping/resource_mapping.openapi.yaml(lines 531-536)Update those
examplesto valid RFC3339 timestamp strings and keep the example block consistent across files.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/openapi/policy/obligations/obligations.openapi.yaml` around lines 726 - 731, The examples for the schema google.protobuf.Timestamp currently use duration literals; update the examples to valid RFC3339 date-time strings (e.g., "2023-01-02T15:04:05Z" and "2023-01-02T15:04:05.000340012Z") and replace the duration examples in the google.protobuf.Timestamp example blocks so all three files (the google.protobuf.Timestamp schema in obligations.openapi.yaml, registered_resources.openapi.yaml, and resource_mapping.openapi.yaml) use the same RFC3339 example values for consistency.
822-857:
⚠️ Potential issue | 🟠 Major🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail files=( "docs/openapi/policy/obligations/obligations.openapi.yaml" "docs/openapi/policy/registeredresources/registered_resources.openapi.yaml" "docs/openapi/policy/resourcemapping/resource_mapping.openapi.yaml" ) for f in "${files[@]}"; do echo "===== $f =====" # Show the relevant region with line numbers (as requested by the original comment) nl -ba "$f" | sed -n '780,900p' | sed -n '1,200p' || true doneRepository: opentdf/platform
Length of output: 437
🏁 Script executed:
#!/bin/bash set -euo pipefail files=( "docs/openapi/policy/obligations/obligations.openapi.yaml" "docs/openapi/policy/registeredresources/registered_resources.openapi.yaml" "docs/openapi/policy/resourcemapping/resource_mapping.openapi.yaml" ) for f in "${files[@]}"; do echo "===== $f =====" awk -v start=780 -v end=900 'NR>=start && NR<=end { printf "%d\t%s\n", NR, $0 }' "$f" | sed -n '1,140p' doneRepository: opentdf/platform
Length of output: 14485
🏁 Script executed:
#!/bin/bash set -euo pipefail # Find and display the policy.Action schema blocks in all 3 files files=( "docs/openapi/policy/obligations/obligations.openapi.yaml" "docs/openapi/policy/registeredresources/registered_resources.openapi.yaml" "docs/openapi/policy/resourcemapping/resource_mapping.openapi.yaml" ) for f in "${files[@]}"; do echo "===== $f (policy.Action occurrences) =====" # Print line numbers where policy.Action appears # Then print ~60 lines around the first few matches matches=$(rg -n "policy\.Action:" "$f" || true) echo "$matches" | head -n 20 if [ -n "$matches" ]; then # Extract first matching line number first_ln=$(echo "$matches" | head -n1 | cut -d: -f1) start=$((first_ln-20)) end=$((first_ln+80)) echo "===== $f (context around first match: lines $start-$end) =====" awk -v start="$start" -v end="$end" 'NR>=start && NR<=end { printf "%d\t%s\n", NR, $0 }' "$f" fi doneRepository: opentdf/platform
Length of output: 13360
🏁 Script executed:
#!/bin/bash set -euo pipefail files=( "docs/openapi/policy/obligations/obligations.openapi.yaml" "docs/openapi/policy/registeredresources/registered_resources.openapi.yaml" "docs/openapi/policy/resourcemapping/resource_mapping.openapi.yaml" ) for f in "${files[@]}"; do echo "===== $f: policy.Action oneOf block around custom/standard =====" # Print from oneOf start to just past parent additionalProperties to spot any additionalProperties in branches ln=$(rg -n "policy\.Action:\s*$" "$f" | head -n1 | cut -d: -f1) start=$((ln)) end=$((ln+80)) awk -v start="$start" -v end="$end" 'NR>=start && NR<=end { print NR "\t" $0 }' "$f" \ | sed -n '1,120p' done echo "===== $f: searching for additionalProperties within +/- 15 lines of custom/standard keys =====" for f in "${files[@]}"; do echo "--- $f ---" rg -n "additionalProperties" "$f" | head -n 30 doneRepository: opentdf/platform
Length of output: 12468
Fix
policy.Actionschema socustom/standardaren’t rejected byadditionalProperties: false.In all three files’
policy.Actionschemas,customandstandardare defined only inside theoneOfbranches, while the parent object setsadditionalProperties: falseand only listsid,name,namespace, andmetadata. JSON Schema validators will treatcustom/standardas additional (unknown) properties and fail validation. Movecustomandstandardinto the parentproperties(and keeponeOfto enforce exactly one variant) or otherwise relax/adjustadditionalProperties. (json-schema.org)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/openapi/policy/obligations/obligations.openapi.yaml` around lines 822 - 857, The policy.Action schema currently defines custom and standard only inside the oneOf branches while the parent object sets additionalProperties: false and lacks those properties, causing validators to reject them; fix by adding top-level properties entries for "custom" and "standard" in the policy.Action properties block (matching the types/$ref used in the oneOf branches) and keep the existing oneOf to enforce which variant is present (or adjust the oneOf to reference the top-level properties), ensuring additionalProperties: false no longer blocks the defined fields; update the policy.Action schema (symbols: policy.Action, custom, standard, additionalProperties, oneOf) accordingly in all three files.docs/openapi/policy/subjectmapping/subject_mapping.openapi.yaml (3)
593-598:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
google.protobuf.Timestampexamples are invalid indocs/openapi/policy/subjectmapping/subject_mapping.openapi.yamlanddocs/openapi/policy/unsafe/unsafe.openapi.yaml. Both files still declaregoogle.protobuf.Timestampas astringwithformat: date-timeand describe RFC 3339 JSON encoding, but the new examples are duration literals (1s,1.000340012s). That makes the published examples contradict the schema and can trip example-aware validators or generated docs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/openapi/policy/subjectmapping/subject_mapping.openapi.yaml` around lines 593 - 598, The examples for the schema named google.protobuf.Timestamp are invalid duration literals; update the examples in docs/openapi/policy/subjectmapping/subject_mapping.openapi.yaml (and the same change in docs/openapi/policy/unsafe/unsafe.openapi.yaml) so they are valid RFC 3339 date-time strings that match the declared type/format (e.g., "2023-01-01T00:00:00Z", "2023-01-01T12:34:56.789Z") instead of "1s"/"1.000340012s", ensuring the google.protobuf.Timestamp entry remains type: string and format: date-time.
689-724:
⚠️ Potential issue | 🟠 Major🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail files=( "docs/openapi/policy/subjectmapping/subject_mapping.openapi.yaml" "docs/openapi/policy/unsafe/unsafe.openapi.yaml" ) for f in "${files[@]}"; do echo "==== $f ====" # show the exact cited ranges if file is long enough for r in "689-724" "596-631" "1087-1113" "1046-1073"; do start=${r%-*}; end=${r#*-} if [ -f "$f" ]; then total=$(wc -l < "$f") if [ "$total" -ge "$end" ]; then echo "-- lines $r --" sed -n "${start},${end}p" "$f" else echo "-- lines $r (skipped; file only $total lines) --" fi fi done echo # locate schema definitions by name echo "-- occurrences: policy.Action / policy.PublicKey --" rg -n "policy\.Action|policy\.PublicKey" "$f" || true echo # Extract policy.Action and policy.PublicKey blocks (best-effort via line windows) echo "-- block windows around policy.Action and policy.PublicKey --" for sym in "policy.Action" "policy.PublicKey"; do line=$(rg -n "$sym" "$f" | head -n1 | cut -d: -f1 || true) if [ -n "$line" ]; then start=$((line-20)); end=$((line+140)) echo "-- $sym around line $line (show $start-$end) --" sed -n "${start},${end}p" "$f" else echo "-- $sym not found (or rg pattern didn't match) --" fi done echo "==== end $f ====" echo doneRepository: opentdf/platform
Length of output: 31923
Fix schema unsatisfiability from
oneOf-only fields + parentadditionalProperties: false.
- In both
docs/openapi/policy/subjectmapping/subject_mapping.openapi.yamlanddocs/openapi/policy/unsafe/unsafe.openapi.yaml,components/schemas/policy.Actionandcomponents/schemas/policy.PublicKeydefine the variant fields only insideoneOfbranches (custom/standard,cached/remote) while the parent object setsadditionalProperties: false, so instances that satisfy aoneOfbranch are still rejected by the parent-level guard.- Move those variant properties onto the parent
properties, or replace the parent guard withunevaluatedProperties: falsesooneOf-validated properties aren’t treated as additional.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/openapi/policy/subjectmapping/subject_mapping.openapi.yaml` around lines 689 - 724, The schemas policy.Action and policy.PublicKey are unsatisfiable because their variant fields live only inside oneOf branches while the parent sets additionalProperties: false; fix by either (A) moving the variant properties (for policy.Action: custom, standard; for policy.PublicKey: cached, remote) into the parent properties block and keep the oneOf for required/validation, or (B) replace the parent-level additionalProperties: false with unevaluatedProperties: false so properties validated by oneOf branches are not treated as "additional"; update the policy.Action and policy.PublicKey schema blocks accordingly and retain existing oneOf titles/required constraints.
1821-1866:
⚠️ Potential issue | 🟠 Major🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the referenced YAML regions for f in \ "docs/openapi/policy/subjectmapping/subject_mapping.openapi.yaml" \ "docs/openapi/policy/unsafe/unsafe.openapi.yaml" \ "docs/openapi/wellknownconfiguration/wellknown_configuration.openapi.yaml" do echo "===== $f =====" # Print the specified line ranges (from the review comment) case "$f" in *subject_mapping.openapi.yaml) sed -n '1800,1880p' "$f" ;; *unsafe.openapi.yaml) sed -n '1630,1710p' "$f" ;; *wellknown_configuration.openapi.yaml) sed -n '120,240p' "$f" ;; *) sed -n '1,120p' "$f" ;; esac done # Global search for connect error schema fields across repo echo "===== rg for connect error details fields =====" rg -n --hidden --no-ignore-vcs "connect\.error|google\.rpc\.Status\.details|details:\s|\bdetail:\s" docs/openapi -S # Search for "detail:" under connect.error specifically echo "===== rg for "connect.error" blocks containing detail(s) =====" rg -n --hidden --no-ignore-vcs -g'*.yaml' "connect\.error:" docs/openapi -S # Extract any schema snippets defining the field names "detail" or "details" near connect.error # Limit output by filtering to the key lines echo "===== rg for "detail:" "details:" near connect.error in each file =====" for f in \ "docs/openapi/policy/subjectmapping/subject_mapping.openapi.yaml" \ "docs/openapi/policy/unsafe/unsafe.openapi.yaml" \ "docs/openapi/wellknownconfiguration/wellknown_configuration.openapi.yaml" do echo "---- $f ----" # show lines containing 'connect.error:' and nearby 'detail'/'details' lines rg -n "connect\.error:|detail:|details:" "$f" -n || true doneRepository: opentdf/platform
Length of output: 32356
🏁 Script executed:
#!/bin/bash set -euo pipefail # Find any `details:` key usages in OpenAPI YAMLs (to see if any connect.error uses plural) rg -n --hidden --no-ignore-vcs -g'*.yaml' '\bdetails\s*:' docs/openapi -S | head -n 200Repository: opentdf/platform
Length of output: 227
🌐 Web query:
ConnectRPC HTTP error representation field name detail vs details google.rpc.Status.details💡 Result:
In the ConnectRPC protocol, the field used to represent error details is named "details" (plural), which is consistent with the google.rpc.Status definition [1][2][3][4]. There is no "detail" field in the standard ConnectRPC error representation [1][5][6]. Key points regarding the error representation: 1. Consistency with google.rpc.Status: ConnectRPC follows the google.rpc.Status error model [2][3][4]. The Status message structure includes a "details" field, which is a repeated list of google.protobuf.Any messages [2][4]. 2. HTTP Representation: When errors are transmitted over HTTP in the Connect protocol, they are serialized as a JSON object [1]. This JSON object includes a "details" array [1][6]. Each item in this array typically contains "type" and "value" fields, and may optionally include a "debug" field (which provides a human-readable representation of the error detail) [1][5][6]. 3. Programming Interfaces: Across ConnectRPC SDKs (e.g., Go, Python, Rust), the interface for accessing these errors consistently uses "details" (e.g., a Details method or a details attribute) to retrieve or manipulate this list [1][7][8][3][9]. In summary, the correct term is always "details" (plural), both in the protocol specification's JSON output and within the library code implementations [1][7][8][4].
Citations:
- 1: https://connectrpc.com/docs/python/errors
- 2: https://github.com/connectrpc/connect-go/blob/2b9d2f6efbf6070ea4e39b2562557beeb455c194/internal/proto/connectext/grpc/status/v1/status.proto
- 3: https://github.com/connectrpc/connect-go/blob/e9aff4ae/internal/gen/connectext/grpc/status/v1/status.pb.go
- 4: https://github.com/googleapis/api-common-protos/blob/87185dfffad4afa5a33a8c153f0e1ea53b4f85dc/google/rpc/status.proto
- 5: https://docs.rs/connectrpc-axum-core/latest/connectrpc_axum_core/struct.ErrorDetail.html
- 6: https://docs.tcn.com/connect-error-6575697d0
- 7: https://github.com/connectrpc/connect-go/blob/main/error.go
- 8: https://docs.rs/connectrpc-axum-core/latest/connectrpc_axum_core/struct.Status.html
- 9: https://connect-python.readthedocs.io/en/latest/_modules/connectrpc/errors.html
🏁 Script executed:
#!/bin/bash set -euo pipefail # Show the surrounding lines for the `details:` occurrences for f in \ "docs/openapi/entityresolution/entity_resolution.openapi.yaml" \ "docs/openapi/entityresolution/v2/entity_resolution.openapi.yaml" do echo "===== $f (around details:) =====" rg -n "details:" "$f" | while read -r line; do ln=$(echo "$line" | cut -d: -f1) sed -n "$((ln-20)),$((ln+20))p" "$f" echo "-----" done done # For each entityresolution file, print its connect.error schema block header and nearby fields for f in \ "docs/openapi/entityresolution/entity_resolution.openapi.yaml" \ "docs/openapi/entityresolution/v2/entity_resolution.openapi.yaml" do echo "===== $f (connect.error block) =====" # Extract from connect.error: down until google.protobuf.Any (heuristic) awk ' /connect\.error:/ {inblock=1} inblock {print} inblock && /google\.protobuf\.Any:/ {exit} ' "$f" | sed -n '1,200p' echo "-----" doneRepository: opentdf/platform
Length of output: 5210
🏁 Script executed:
#!/bin/bash set -euo pipefail # Collect files with connect.error schema blocks mapfile -t files < <(rg -l --hidden --no-ignore-vcs -g'*.yaml' 'connect\.error:' docs/openapi) echo "Found ${`#files`[@]} files with connect.error:" for f in "${files[@]}"; do echo "---- $f ----" # Determine whether `detail:` or `details:` appears inside the connect.error block # Heuristic: start at connect.error:, end at next top-level component schema (2-space indent + word + :) or end of file. awk ' function trim(s){gsub(/^[ \t]+|[ \t]+$/,"",s); return s} /connect\.error:/ {in=1} in { # detect key names at indentation >= 8 (common in schemas) if ($0 ~ /^[[:space:]]*details[[:space:]]*:/) has_details=1 if ($0 ~ /^[[:space:]]*detail[[:space:]]*:/) has_detail=1 # crude end: next schema name at same indent as connect.error (4 spaces) followed by colon if (NR>1 && $0 ~ /^[[:space:]]{4}[A-Za-z0-9_.-]+[[:space:]]*:/) { if ($0 !~ /connect\.error:/) {in=0} } } END{ if (has_details) print "connect.error contains: details:" if (has_detail) print "connect.error contains: detail:" if (!has_details && !has_detail) print "connect.error contains neither detail: nor details:" } ' "$f" done # Also, any global plural usage in connect.error specifically (sanity) echo "===== Global check: connect.error block lines containing details: =====" rg -n --hidden --no-ignore-vcs -g'*.yaml' 'connect\.error:|^\s*details\s*:' docs/openapi | head -n 200Repository: opentdf/platform
Length of output: 117
🏁 Script executed:
#!/bin/bash set -euo pipefail # List all yaml files containing connect.error schema blocks rg -l --hidden --no-ignore-vcs -g'*.yaml' 'connect\.error:' docs/openapi > /tmp/connect_error_files.txt mapfile -t files < /tmp/connect_error_files.txt echo "Found ${`#files`[@]} files with connect.error:" printf '%s\n' "${files[@]}" echo echo "===== Per-file: whether connect.error uses detail vs details =====" for f in "${files[@]}"; do # Take only the connect.error block by cutting from "connect.error:" until "google.protobuf.Any:" (if present), # otherwise until the next top-level schema key (heuristic). block="$(awk ' /connect\.error:/ {in=1} in { print # stop near end of connect.error schema block if ($0 ~ /google\.protobuf\.Any:/) {exit} # fallback end: next schema block at same indent (very heuristic) if (NR>1 && $0 ~ /^[[:space:]]{4}[A-Za-z0-9_.-]+[[:space:]]*:/ && $0 !~ /connect\.error:/) {exit} } ' "$f")" has_detail="$(printf '%s\n' "$block" | rg -n '^[[:space:]]*detail[[:space:]]*:' -m1 || true)" has_details="$(printf '%s\n' "$block" | rg -n '^[[:space:]]*details[[:space:]]*:' -m1 || true)" if [[ -n "$has_details" ]]; then echo "DETAILS : $f" elif [[ -n "$has_detail" ]]; then echo "DETAIL : $f" else echo "NEITHER : $f" fi done echo echo "===== Global check: lines with connect.error or detail(s): =====" rg -n --hidden --no-ignore-vcs -g'*.yaml' '(connect\.error:|^\s*details\s*:|^\s*detail\s*:)' docs/openapi | head -n 200Repository: opentdf/platform
Length of output: 1228
🏁 Script executed:
#!/bin/bash set -euo pipefail rg -l --hidden --no-ignore-vcs -g'*.yaml' 'connect\.error:' docs/openapi > /tmp/connect_error_files.txt mapfile -t files < /tmp/connect_error_files.txt echo "Found ${`#files`[@]} files with connect.error:" printf '%s\n' "${files[@]}" echo echo "===== Per-file: whether connect.error uses detail vs details =====" for f in "${files[@]}"; do # Extract from connect.error: until (first) google.protobuf.Any: line, or until next top-level schema key block="$(awk ' /connect\.error:/ {inblock=1} inblock {print} inblock && /google\.protobuf\.Any:/ {exit} inblock && NR>1 && $0 ~ /^[[:space:]]{4}[A-Za-z0-9_.-]+[[:space:]]*:/ && $0 !~ /connect\.error:/ {exit} ' "$f")" if printf '%s\n' "$block" | rg -q '^[[:space:]]*details[[:space:]]*:'; then echo "DETAILS : $f" elif printf '%s\n' "$block" | rg -q '^[[:space:]]*detail[[:space:]]*:'; then echo "DETAIL : $f" else echo "NEITHER : $f" fi done echo echo "===== Global check: lines with connect.error or detail(s): (head) =====" rg -n --hidden --no-ignore-vcs -g'*.yaml' '(connect\.error:|^\s*details\s*:|^\s*detail\s*:)' docs/openapi | head -n 200Repository: opentdf/platform
Length of output: 5064
Fix OpenAPI
connect.errorschema: it modelsdetail(singular) but ConnectRPC usesdetails(collection), makinggoogle.rpc.Status.detailsinconsistent.
connect.errorindocs/openapi/policy/subjectmapping/subject_mapping.openapi.yaml(1821-1866),docs/openapi/policy/unsafe/unsafe.openapi.yaml(1653-1698), anddocs/openapi/wellknownconfiguration/wellknown_configuration.openapi.yaml(152-197) definesdetail: google.protobuf.Anywhile the descriptions referencegoogle.rpc.Status.details. Update the contract to usedetailsas an array/collection (matching ConnectRPC/google.rpc.Status.details).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/openapi/policy/subjectmapping/subject_mapping.openapi.yaml` around lines 1821 - 1866, The connect.error schema currently defines a singular property named "detail" of type google.protobuf.Any but must match ConnectRPC/google.rpc.Status which uses a repeated "details" field; update the connect.error schema (the connect.error component) to replace the "detail" property with "details" as an array of google.protobuf.Any (e.g., type: array, items: $ref: '`#/components/schemas/google.protobuf.Any`'), keep additionalProperties/title/description unchanged and ensure any references or examples that mention google.rpc.Status.details remain consistent with the new plural field name.service/policy/selectors_test.go (1)
12-25: 🧹 Nitpick | 🔵 Trivial | 💤 Low value
Consider expanding test coverage for edge cases.
The test correctly validates the 253-character boundary, but additional test cases would improve robustness:
- Empty string (should pass)
- Whitespace-only strings
- Unicode/multibyte characters (ensure byte vs. character length handling)
- Strings with special characters
🧪 Example additional test cases
func TestSearchTermValidation(t *testing.T) { v, err := protovalidate.New() require.NoError(t, err) require.NoError(t, v.Validate(&policy.Search{ Term: strings.Repeat("a", 253), })) err = v.Validate(&policy.Search{ Term: strings.Repeat("a", 254), }) require.Error(t, err) require.Contains(t, err.Error(), "string.max_len") + + // Empty string should be valid + require.NoError(t, v.Validate(&policy.Search{ + Term: "", + })) + + // Test unicode handling + require.NoError(t, v.Validate(&policy.Search{ + Term: strings.Repeat("☃", 253), + })) }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.func TestSearchTermValidation(t *testing.T) { v, err := protovalidate.New() require.NoError(t, err) require.NoError(t, v.Validate(&policy.Search{ Term: strings.Repeat("a", 253), })) err = v.Validate(&policy.Search{ Term: strings.Repeat("a", 254), }) require.Error(t, err) require.Contains(t, err.Error(), "string.max_len") // Empty string should be valid require.NoError(t, v.Validate(&policy.Search{ Term: "", })) // Test unicode handling require.NoError(t, v.Validate(&policy.Search{ Term: strings.Repeat("☃", 253), })) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@service/policy/selectors_test.go` around lines 12 - 25, Extend TestSearchTermValidation to cover edge cases: add assertions that an empty Term and a whitespace-only Term pass validation, that multibyte/unicode strings are validated according to the intended length semantics (add a test using runes vs bytes to confirm whether policy.Search.Term limit applies to characters or bytes), and that strings containing special characters pass/fail appropriately; update tests that call protovalidate.New() and v.Validate(&policy.Search{Term: ...}) to include these new cases and assert success or string.max_len error as appropriate.service/policy/selectors.proto (1)
67-69: 🧹 Nitpick | 🔵 Trivial | 💤 Low value
Consider documenting the rationale for the 253 character limit.
The max_len constraint of 253 characters aligns with DNS FQDN limits, but documenting why this specific value was chosen would help future maintainers understand the design decision, especially given the PR description mentions "substring matching in Postgres" where text field limits are typically much higher.
📝 Suggested documentation addition
message Search { + // Maximum length of 253 characters (DNS FQDN limit) string term = 1 [(buf.validate.field).string.max_len = 253]; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.message Search { // Maximum length of 253 characters (DNS FQDN limit) string term = 1 [(buf.validate.field).string.max_len = 253]; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@service/policy/selectors.proto` around lines 67 - 69, Add a short comment above the Search.message or the term field explaining why the string max_len is set to 253 (e.g., to match DNS FQDN limits used by the system and to constrain substring matching semantics in Postgres), so future maintainers see the rationale; reference the Search message and the term field with the (buf.validate.field).string.max_len = 253 constraint in the comment.
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
|
Contributor
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
This was referenced Jun 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
1.) Add common search object that will be used for substring matching in Postgres.
Summary by CodeRabbit
New Features
Documentation