-
Notifications
You must be signed in to change notification settings - Fork 41
feat: validate resourceDependencies at write time, gated by allowUserResourceDependencies #1941
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
serguei-gorokhov
wants to merge
3
commits into
rd/1-definition-model
Choose a base branch
from
rd/2-write-time-validation
base: rd/1-definition-model
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
196 changes: 196 additions & 0 deletions
196
server/src/main/java/com/epam/aidial/core/server/service/ResourceDependencyValidator.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,196 @@ | ||
| package com.epam.aidial.core.server.service; | ||
|
|
||
| import com.epam.aidial.core.config.Application; | ||
| import com.epam.aidial.core.config.ResourceAccessType; | ||
| import com.epam.aidial.core.config.ResourceDependency; | ||
| import com.epam.aidial.core.storage.http.HttpException; | ||
| import com.epam.aidial.core.storage.util.UrlUtil; | ||
| import lombok.RequiredArgsConstructor; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.Arrays; | ||
| import java.util.HashSet; | ||
| import java.util.List; | ||
| import java.util.Set; | ||
|
|
||
| import static com.epam.aidial.core.storage.http.HttpStatus.BAD_REQUEST; | ||
| import static com.epam.aidial.core.storage.http.HttpStatus.FORBIDDEN; | ||
|
|
||
| /** | ||
| * Write-time validation of the {@code resourceDependencies} declaration section. This is the | ||
| * pointer rule, not an access decision: creating a dependency requires no permission on the | ||
| * target — whether the originating user can reach the target is a runtime question, verified | ||
| * fresh per request at resolution time. Only the shape of the ask and the authoring governance | ||
| * ceiling are checked here. | ||
| */ | ||
| @RequiredArgsConstructor | ||
| public class ResourceDependencyValidator { | ||
|
|
||
| /** A declaration larger than this is wrong-shaped; it should be folder-scoped links, not a file inventory. */ | ||
| public static final int MAX_DECLARED_DEPENDENCIES = 100; | ||
|
|
||
| public static final String CURRENT_USER_PLACEHOLDER = "current-user"; | ||
|
|
||
| /** Global-view roots a concrete path may address. The personal root is reachable only via the placeholder. */ | ||
| private static final Set<String> GLOBAL_VIEW_ROOTS = | ||
| Set.of("files", "public", "prompts", "conversations", "applications", "toolsets", "skills"); | ||
|
|
||
| /** | ||
| * Resource-type folders a {@code current-user/…} path must be rooted in for user-authored apps — | ||
| * a root-level {@code current-user/} declaration ("write everything personal") is not declarable. | ||
| */ | ||
| private static final Set<String> PERSONAL_TYPED_ROOTS = | ||
| Set.of("files", "prompts", "conversations", "applications", "toolsets", "skills"); | ||
|
|
||
| private final boolean allowUserResourceDependencies; | ||
|
|
||
| /** Throws on the first shape violation. Applied on every writer surface regardless of author. */ | ||
| public void validateShape(Application application) { | ||
| List<String> issues = shapeIssues(application); | ||
| if (!issues.isEmpty()) { | ||
| throw new HttpException(BAD_REQUEST, "Invalid resource dependencies: " + String.join("; ", issues)); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Governance ceiling for user-authored apps: with the flag off (the default) they may not declare | ||
| * dependencies at all; with it on, personal targets must be typed — never the personal root. | ||
| * Admin-authored writes (public bucket by an admin, the platform bucket) are not gated here. | ||
| */ | ||
| public void validateUserAuthored(Application application) { | ||
| List<ResourceDependency> section = application.getResourceDependencies(); | ||
| if (section == null || section.isEmpty()) { | ||
| return; | ||
| } | ||
| if (!allowUserResourceDependencies) { | ||
| throw new HttpException(FORBIDDEN, | ||
| "User-authored applications may not declare resource dependencies (allowUserResourceDependencies is disabled)"); | ||
| } | ||
| for (ResourceDependency dependency : section) { | ||
| String path = pathOf(dependency); | ||
| if (path == null) { | ||
| continue; | ||
| } | ||
| String[] segments = decodedSegments(path); | ||
| if (segments.length > 0 && CURRENT_USER_PLACEHOLDER.equals(segments[0]) && !isTypedPersonalPath(segments)) { | ||
| throw new HttpException(FORBIDDEN, "Root-level current-user dependency is not declarable: " | ||
| + "personal targets must be rooted in a resource-type folder: " + path); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** Non-throwing form of {@link #validateShape}: the same rules, usable from any write surface. */ | ||
| public static List<String> shapeIssues(Application application) { | ||
| List<String> issues = new ArrayList<>(); | ||
| List<ResourceDependency> section = application.getResourceDependencies(); | ||
| if (section == null || section.isEmpty()) { | ||
| return issues; | ||
| } | ||
| boolean overCap = section.size() > MAX_DECLARED_DEPENDENCIES; | ||
| if (overCap) { | ||
| issues.add("resourceDependencies: the section exceeds " + MAX_DECLARED_DEPENDENCIES + " entries"); | ||
| } | ||
| // Once over the cap the section is rejected anyway — inspect only the first MAX entries so a | ||
| // huge body cannot turn validation itself into unbounded allocation. | ||
| int inspected = Math.min(section.size(), MAX_DECLARED_DEPENDENCIES); | ||
| Set<String> seenLinkIds = new HashSet<>(); | ||
| for (int i = 0; i < inspected; i++) { | ||
| ResourceDependency dependency = section.get(i); | ||
| String at = "resourceDependencies[" + i + "]"; | ||
| if (dependency == null) { | ||
| issues.add(at + ": entry is null"); | ||
| continue; | ||
| } | ||
| if (!ResourceDependency.KIND.equals(dependency.getKind())) { | ||
| issues.add(at + ": kind must be " + ResourceDependency.KIND); | ||
| } | ||
| String linkId = dependency.getLinkId(); | ||
| if (linkId == null || linkId.isBlank()) { | ||
| issues.add(at + ": linkId is required"); | ||
| } else if (!seenLinkIds.add(linkId)) { | ||
| issues.add(at + ": duplicate linkId '" + linkId + "'"); | ||
| } | ||
| issues.addAll(pathIssues(at, dependency)); | ||
| // An explicit JSON null defeats the field default, so guard for null alongside empty. | ||
| if (dependency.getAccess() == null || dependency.getAccess().isEmpty()) { | ||
| issues.add(at + ": access must not be empty"); | ||
| } else if (dependency.getAccess().contains(ResourceAccessType.SHARE)) { | ||
| issues.add(at + ": SHARE is not a dependency right"); | ||
| } | ||
| } | ||
| return issues; | ||
| } | ||
|
|
||
| private static List<String> pathIssues(String at, ResourceDependency dependency) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the logic should rely on ResourceDescriptorFactory |
||
| List<String> issues = new ArrayList<>(); | ||
| String path = pathOf(dependency); | ||
| if (path == null) { | ||
| issues.add(at + ": target.path is required"); | ||
| return issues; | ||
| } | ||
| // Token rules run on decoded segments, mirroring ResourceDescriptorFactory's single tryDecodePath | ||
| // pass — the platform canonicalizes declared paths through that decode, so validating the raw | ||
| // string would let %2e%2e / %2a / %63urrent-user smuggle banned tokens past the bans. | ||
| String[] segments = decodedSegments(path); | ||
| // A path of slashes only splits to zero segments; treat it as a missing path, not a crash. | ||
| if (segments.length == 0) { | ||
| issues.add(at + ": target.path is required"); | ||
| return issues; | ||
| } | ||
| String root = segments[0]; | ||
| // Token rules on every segment after the root; the root itself is governed by the form checks below. | ||
| for (int i = 1; i < segments.length; i++) { | ||
| String segment = segments[i]; | ||
| if (CURRENT_USER_PLACEHOLDER.equals(segment)) { | ||
| issues.add(at + ": the current-user placeholder is valid only as the root segment: " + path); | ||
| } | ||
| if (segment.isEmpty()) { | ||
| issues.add(at + ": path must not contain empty segments: " + path); | ||
| } | ||
| if (segment.contains("*")) { | ||
| issues.add(at + ": wildcards are not allowed: " + path); | ||
| } | ||
| if (".".equals(segment) || "..".equals(segment)) { | ||
| issues.add(at + ": relative path segments are not allowed: " + path); | ||
| } | ||
| } | ||
| if (CURRENT_USER_PLACEHOLDER.equals(root)) { | ||
| // Placeholder-rooted form; the typed-root restriction is the governance ceiling's, not shape's. | ||
| return issues; | ||
| } | ||
| if ("users".equals(root)) { | ||
| // Personal targets are declared only via the placeholder — a concrete users/… path resolves for | ||
| // no one but that user and is rejected at write time as a shape error. | ||
| issues.add(at + ": personal targets must use the current-user placeholder, not a concrete users/… path: " + path); | ||
| } else if (!GLOBAL_VIEW_ROOTS.contains(root)) { | ||
| issues.add(at + ": target must be a global-view path or current-user rooted: " + path); | ||
| } else if (segments.length < 2) { | ||
| // A bare type root addresses the whole global view of that type — as over-broad as the | ||
| // personal root the governance ceiling bans. Declarations must be folder- or file-scoped. | ||
| issues.add(at + ": target must address a folder or resource within " + root + "/, not the type root: " + path); | ||
| } | ||
| return issues; | ||
| } | ||
|
|
||
| private static boolean isTypedPersonalPath(String[] segments) { | ||
| return segments.length > 1 && PERSONAL_TYPED_ROOTS.contains(segments[1]); | ||
| } | ||
|
|
||
| private static String pathOf(ResourceDependency dependency) { | ||
| if (dependency.getTarget() == null || dependency.getTarget().getPath() == null) { | ||
| return null; | ||
| } | ||
| String path = dependency.getTarget().getPath().trim(); | ||
| return path.isEmpty() ? null : path; | ||
| } | ||
|
|
||
| /** Splits off a single trailing slash (folder targets end with one) before splitting into segments. */ | ||
| private static String[] splitPath(String path) { | ||
| String trimmed = path.endsWith("/") ? path.substring(0, path.length() - 1) : path; | ||
| return trimmed.split("/"); | ||
| } | ||
|
|
||
| private static String[] decodedSegments(String path) { | ||
| return Arrays.stream(splitPath(path)).map(UrlUtil::tryDecodePath).toArray(String[]::new); | ||
| } | ||
| } | ||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to call the min function?
We already know the size is less or equal to MAX cap