Skip to content

Commit

Permalink
[ENG-7120] fix calling internal functions from dashboard (#28973)
Browse files Browse the repository at this point in the history
when components are enabled, we check that function paths are public exports, and if they're not we throw an error early.
this is a problem for the dashboard which should be allowed to call internal functions, but they're not public exports.
therefore we should have a fallback to the udf path if the function path isn't a public export. we still validate later on that the function is public if the caller is only allowed to call public functions. this is falling back to the legacy behavior where exports aren't special.

tested in the dashboard and you can now call internal functions again. also tested with `npx convex run` for good measure (it can call internal functions). and tested with a non-admin client to see that it gives the expected error:

```
Could not find public function for 'messages:sumNumbers'. Did you forget to run `npx convex dev` or `npx convex deploy`?
```

GitOrigin-RevId: df6416e75f90730836d3215511df045f89bbb942
  • Loading branch information
ldanilek authored and Convex, Inc. committed Aug 16, 2024
1 parent 610c156 commit e19e4fa
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 28 deletions.
36 changes: 13 additions & 23 deletions crates/isolate/src/environment/helpers/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -316,41 +316,31 @@ impl ValidatedPathAndArgs {
},
}

let maybe_path = match public_path.clone() {
let path = match public_path.clone() {
PublicFunctionPath::RootExport(path) => {
match ComponentsModel::new(tx)
let path = ComponentsModel::new(tx)
.resolve_public_export_path(path)
.await?
{
Some(path) => {
let (_, component) = BootstrapComponentsModel::new(tx)
.component_path_to_ids(path.component.clone())
.await?;
Some(ResolvedComponentFunctionPath {
component,
udf_path: path.udf_path,
component_path: Some(path.component),
})
},
None => None,
.await?;
let (_, component) = BootstrapComponentsModel::new(tx)
.component_path_to_ids(path.component.clone())
.await?;
ResolvedComponentFunctionPath {
component,
udf_path: path.udf_path,
component_path: Some(path.component),
}
},
PublicFunctionPath::Component(path) => {
let (_, component) = BootstrapComponentsModel::new(tx)
.component_path_to_ids(path.component.clone())
.await?;
Some(ResolvedComponentFunctionPath {
ResolvedComponentFunctionPath {
component,
udf_path: path.udf_path,
component_path: Some(path.component),
})
}
},
PublicFunctionPath::ResolvedComponent(path) => Some(path),
};
let Some(path) = maybe_path else {
return Ok(Err(JsError::from_message(missing_or_internal_error(
public_path.clone(),
)?)));
PublicFunctionPath::ResolvedComponent(path) => path,
};

let udf_version = match udf_version(&path, tx).await? {
Expand Down
15 changes: 10 additions & 5 deletions crates/model/src/components/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,29 +223,34 @@ impl<'a, RT: Runtime> ComponentsModel<'a, RT> {
pub async fn resolve_public_export_path(
&mut self,
path: ExportPath,
) -> anyhow::Result<Option<CanonicalizedComponentFunctionPath>> {
) -> anyhow::Result<CanonicalizedComponentFunctionPath> {
let root_definition = BootstrapComponentsModel::new(self.tx)
.load_definition(ComponentDefinitionId::Root)
.await?;
// Legacy path: If components aren't enabled, just resolve export paths directly
// to UDF paths.
if root_definition.is_none() {
return Ok(Some(CanonicalizedComponentFunctionPath {
return Ok(CanonicalizedComponentFunctionPath {
component: ComponentPath::root(),
udf_path: path.into(),
}));
});
}
let path_components = path.components();
let resource = self
.resolve_export(ComponentId::Root, &path_components)
.await?;
let Some(resource) = resource else {
return Ok(None);
// In certain cases non-exported functions can be called, e.g.
// system auth can call internal functions.
return Ok(CanonicalizedComponentFunctionPath {
component: ComponentPath::root(),
udf_path: path.into(),
});
};
let Resource::Function(path) = resource else {
anyhow::bail!("Expected a function");
};
Ok(Some(path))
Ok(path)
}

pub async fn preload_resources(
Expand Down

0 comments on commit e19e4fa

Please sign in to comment.