Skip to content

fix: move the target-path placeholder into the bucket slot, drop the dead public root #1946

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.

Sign up for GitHub

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 1 commit into
base: rd/6-consent-status
Choose a base branch
from
Open

fix: move the target-path placeholder into the bucket slot, drop the dead public root #1946

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 14 additions & 14 deletions server/src/main/java/com/epam/aidial/core/server/function/ResolveResourceDependenciesFn.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,9 @@ private static Set<ResourceAccessType> requestedAccessOf(@Nullable ResourceDepen
}

/**
* Resolves a declared target path to a descriptor: a {@code current-user/<type>/<path…>}
* path against the originating user's own bucket, a concrete global-view path as-is. Null
* when the record is malformed — an unresolvable record, never a crash.
* Resolves a declared target path to a descriptor: a {@code {type}/{current-user}/<path…>}
* path against the originating user's own bucket, a concrete {@code {type}/{bucket}/<path…>}
* path as-is. Null when the record is malformed — an unresolvable record, never a crash.
*/
@Nullable
private ResourceDescriptor resolveTarget(ResourceDependency dependency, AuthBucket userBucket) {
Expand All @@ -206,19 +206,19 @@ private ResourceDescriptor resolveTarget(ResourceDependency dependency, AuthBuck
boolean folder = path.endsWith("/");
try {
String[] segments = decodedSegments(path, folder);
if (segments.length == 0) {
if (segments.length < 2) {
return null; // {type}/{bucket} is the minimum
}
if (!ResourceDependencyValidator.DECLARABLE_TYPE_ROOTS.contains(segments[0])) {
// ResourceTypes.of() also maps internal engine types (credentials, keys, models, …).
// Config-file apps bypass write-time validation, so the read side enforces the same
// closed vocabulary — for concrete paths too, not only placeholder ones.
return null;
}
if (ResourceDependencyValidator.CURRENT_USER_PLACEHOLDER.equals(segments[0])) {
// current-user/<type-segment>/<path…>. Two segments (current-user/skills/) target the
// type's root folder in the user's bucket — the skill-creator shape. The type segment
// is restricted to the personal typed-root vocabulary: ResourceTypes.of() also maps
// internal engine types (credentials, keys, models, …), and a declaration like
// current-user/credentials/ must never resolve into the user's secret-bearing blobs.
if (segments.length < 2 || !ResourceDependencyValidator.PERSONAL_TYPED_ROOTS.contains(segments[1])) {
return null;
}
ResourceType type = ResourceTypes.of(segments[1]);
if (ResourceDependencyValidator.CURRENT_USER_PLACEHOLDER.equals(segments[1])) {
// {type}/{current-user}/{path…}. Two segments target the type's root folder in the
// user's bucket — the skill-creator shape.
ResourceType type = ResourceTypes.of(segments[0]);
String relativePath = segments.length == 2
? "" : String.join("/", Arrays.asList(segments).subList(2, segments.length));
if (folder && !relativePath.isEmpty()) {
Expand Down
84 changes: 38 additions & 46 deletions server/src/main/java/com/epam/aidial/core/server/service/ResourceDependencyValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,15 @@ 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");
public static final String CURRENT_USER_PLACEHOLDER = "{current-user}";

/**
* 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.
* Also enforced by the resolver on the read side: ResourceTypes.of() maps internal engine types
* (credentials, keys, models, …) that must never be declarable as personal targets.
* The resource types a declaration may address — segment 0 of {type}/{bucket}/{path…}, for
* both the concrete and the placeholder form. Closed per Core version. ResourceTypes.of()
* also maps internal engine types (credentials, keys, models, …); those are never declarable,
* as a personal target or a concrete one. Enforced on both the write and the read side.
*/
public static final Set<String> PERSONAL_TYPED_ROOTS =
public static final Set<String> DECLARABLE_TYPE_ROOTS =
Set.of("files", "prompts", "conversations", "applications", "toolsets", "skills");

private final boolean allowUserResourceDependencies;
Expand All @@ -56,8 +52,11 @@ public void validateShape(Application application) {

/**
* 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.
* dependencies at all. With it on, no further per-path ceiling applies — the grammar itself already
* requires segment 0 to be a declarable type (shape validation runs first, see ResourceController),
* so a root-level "write everything personal" declaration is not expressible under
* {type}/{bucket}/… at all. 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();
Expand All @@ -68,17 +67,6 @@ public void validateUserAuthored(Application application) {
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. */
Expand Down Expand Up @@ -140,11 +128,34 @@ private static List<String> pathIssues(String at, ResourceDependency dependency)
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++) {
// Root vocabulary — unconditional and first, no early return for either form. Deleting the old
// placeholder early-return is deliberate: without this check running unconditionally,
// credentials/{current-user}/… would silently bypass root-vocabulary validation at write time.
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 are declared as {type}/" + CURRENT_USER_PLACEHOLDER
+ "/…, not as a concrete users/… path: " + path);
} else if (!DECLARABLE_TYPE_ROOTS.contains(root)) {
issues.add(at + ": target must start with a declarable resource type "
+ "(files, prompts, conversations, applications, toolsets, skills): " + 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.
// Applies to both forms: {type}/{bucket}/… is the minimum for either.
issues.add(at + ": target must address a folder or resource within " + root + "/, not the type root: " + path);
}
// Token rules on every segment after the root, plus the placeholder-position rule covering
// segment 0 as well — both issues are collected when the placeholder sits at segment 0, since
// the root-vocabulary error above also fires there and two accurate messages beat one conditional.
for (int i = 0; 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 (CURRENT_USER_PLACEHOLDER.equals(segment) && i != 1) {
issues.add(at + ": the " + CURRENT_USER_PLACEHOLDER
+ " placeholder is valid only as the bucket segment (the second segment): " + path);
}
if (i == 0) {
continue;
}
if (segment.isEmpty()) {
issues.add(at + ": path must not contain empty segments: " + path);
Expand All @@ -156,28 +167,9 @@ private static List<String> pathIssues(String at, ResourceDependency dependency)
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;
Expand Down
27 changes: 16 additions & 11 deletions server/src/test/java/com/epam/aidial/core/server/CustomApplicationApiTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1780,22 +1780,22 @@ void testResourceDependenciesSectionWriteTimeValidation() {
// the typed-root restriction belongs to the user-authored ceiling, not to shape.
Response response = send(HttpMethod.PUT, "/v1/applications/public/resource-dependency-app", null,
dependencyAppBody("""
{"kind": "dial.resourceLink", "link_id": "lnk_skills", "target": {"path": "current-user/skills/"}, "access": ["write"], "required": true},
{"kind": "dial.resourceLink", "link_id": "lnk_skills", "target": {"path": "skills/{current-user}/"}, "access": ["write"], "required": true},
{"kind": "dial.resourceLink", "link_id": "lnk_policies", "target": {"path": "files/public/policies/"}, "access": ["read"]}"""),
"authorization", "admin");
verify(response, 200);

// User-authored (own bucket) with the flag off — the default: rejected wholesale.
response = send(HttpMethod.PUT, "/v1/applications/" + bucket + "/resource-dependency-app", null,
dependencyAppBody("""
{"kind": "dial.resourceLink", "link_id": "lnk_skills", "target": {"path": "current-user/skills/"}, "access": ["write"]}"""));
{"kind": "dial.resourceLink", "link_id": "lnk_skills", "target": {"path": "skills/{current-user}/"}, "access": ["write"]}"""));
verify(response, 403);

// The ceiling is keyed on the author, not the destination bucket: an admin prototyping in
// their own bucket authors an admin app — the section is accepted even with the flag off.
response = send(HttpMethod.PUT, "/v1/applications/" + adminBucket() + "/resource-dependency-app", null,
dependencyAppBody("""
{"kind": "dial.resourceLink", "link_id": "lnk_skills", "target": {"path": "current-user/skills/"}, "access": ["write"]}"""),
{"kind": "dial.resourceLink", "link_id": "lnk_skills", "target": {"path": "skills/{current-user}/"}, "access": ["write"]}"""),
"authorization", "admin");
verify(response, 200);

Expand All @@ -1804,8 +1804,8 @@ void testResourceDependenciesSectionWriteTimeValidation() {
{"wrong kind", "{\"kind\": \"dial.resource\", \"link_id\": \"lnk_1\", \"target\": {\"path\": \"files/public/f/\"}, \"access\": [\"read\"]}"},
{"concrete personal path", "{\"kind\": \"dial.resourceLink\", \"link_id\": \"lnk_1\", \"target\": {\"path\": \"users/bob/files/f/\"}, \"access\": [\"read\"]}"},
{"wildcard", "{\"kind\": \"dial.resourceLink\", \"link_id\": \"lnk_1\", \"target\": {\"path\": \"files/public/*/\"}, \"access\": [\"read\"]}"},
{"placeholder off the root",
"{\"kind\": \"dial.resourceLink\", \"link_id\": \"lnk_1\", \"target\": {\"path\": \"files/public/current-user/f/\"}, \"access\": [\"read\"]}"},
{"placeholder off the bucket slot",
"{\"kind\": \"dial.resourceLink\", \"link_id\": \"lnk_1\", \"target\": {\"path\": \"files/public/%7Bcurrent-user%7D/f/\"}, \"access\": [\"read\"]}"},
{"unknown root", "{\"kind\": \"dial.resourceLink\", \"link_id\": \"lnk_1\", \"target\": {\"path\": \"buckets/public/f/\"}, \"access\": [\"read\"]}"},
{"share is not a dependency right",
"{\"kind\": \"dial.resourceLink\", \"link_id\": \"lnk_1\", \"target\": {\"path\": \"files/public/f/\"}, \"access\": [\"share\"]}"},
Expand Down Expand Up @@ -1843,8 +1843,10 @@ private String adminBucket() {
}

/**
* The governance flag on: user-authored apps may declare dependencies, but personal targets
* must be typed — a root-level {@code current-user/} declaration is not declarable.
* The governance flag on: user-authored apps may declare dependencies. No further per-path
* ceiling applies — the grammar itself already requires segment 0 to be a declarable type, so
* a root-level "write everything personal" declaration is not expressible under
* {type}/{bucket}/… at all (shape rejects it with 400, not the ceiling with 403).
*/
public static class AllowUserResourceDependenciesOn extends ResourceBaseTest {

Expand All @@ -1857,18 +1859,21 @@ protected JsonObject additionalSettingsOverrides() {
void testUserAuthoredDependenciesWhileFlagOn() {
Response response = send(HttpMethod.PUT, "/v1/applications/" + bucket + "/resource-dependency-app", null,
dependencyAppBody("""
{"kind": "dial.resourceLink", "link_id": "lnk_skills", "target": {"path": "current-user/skills/"}, "access": ["write"], "required": true}"""));
{"kind": "dial.resourceLink", "link_id": "lnk_skills", "target": {"path": "skills/{current-user}/"}, "access": ["write"], "required": true}"""));
verify(response, 200);

// The old root-level personal shape ({current-user}/ alone) fails shape validation now —
// 400, not the 403 ceiling: the token is out of position and segment 0 is not a type.
response = send(HttpMethod.PUT, "/v1/applications/" + bucket + "/resource-dependency-app", null,
dependencyAppBody("""
{"kind": "dial.resourceLink", "link_id": "lnk_root", "target": {"path": "current-user/"}, "access": ["write"]}"""));
verify(response, 403);
{"kind": "dial.resourceLink", "link_id": "lnk_root", "target": {"path": "{current-user}/"}, "access": ["write"]}"""));
verify(response, 400);

// The old shipped spelling fails loudly too — current-user is not a declarable type.
response = send(HttpMethod.PUT, "/v1/applications/" + bucket + "/resource-dependency-app", null,
dependencyAppBody("""
{"kind": "dial.resourceLink", "link_id": "lnk_untyped", "target": {"path": "current-user/rootstuff/"}, "access": ["write"]}"""));
verify(response, 403);
verify(response, 400);
}
}

Expand Down
2 changes: 1 addition & 1 deletion server/src/test/java/com/epam/aidial/core/server/PlatformAppToolsetApiTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ void testMalformedResourceDependenciesSectionRejected() {
"endpoint": "http://application1/v1/completions",
"display_name": "Platform App",
"resource_dependencies": [
{"kind": "dial.resourceLink", "link_id": "lnk_1", "target": {"path": "current-user/skills/"}, "access": ["write"]}
{"kind": "dial.resourceLink", "link_id": "lnk_1", "target": {"path": "skills/{current-user}/"}, "access": ["write"]}
]
}
""";
Expand Down
Loading