jerryshao opened a new issue, #13371:
URL: https://github.com/apache/gravitino/issues/13371
### Version
main branch
### Describe what's wrong
Job template names are only required to be non-blank
(`JobTemplateDTO#validate`) and not to start with `builtin-`
(`JobTemplateValidationDispatcher`). `JobManager` builds a job's staging
directory by string concatenation, without normalizing the result or checking
that it stays under `gravitino.job.stagingDir`:
```java
stagingDir.getAbsolutePath() + String.format(JOB_STAGING_DIR, metalake,
jobTemplateName, jobId)
// JOB_STAGING_DIR = "/%s/%s/job-%s"
```
So a template named `../../escape` resolves to
`<stagingDir>/<metalake>/../../escape/job-<id>`, i.e. `<parent of
stagingDir>/escape/job-<id>`. Once `<stagingDir>/<metalake>` exists (after any
other job ran in that metalake), running a job from this template:
- creates the job directory outside the staging directory, and downloads the
template's executable, scripts, jars, files and archives into it (`runJob` /
`createRuntimeJobTemplate`);
- runs the job there, with the local job executor writing `output.log` /
`error.log` into it;
- later deletes that outside directory recursively, in `deleteJobTemplate`
and `cleanUpStagingDirs`.
If `<stagingDir>/<metalake>` doesn't exist yet, the directory is still
created outside and the executable is still downloaded there, but the job fails
to start and the directory is never cleaned up.
Whoever can register a job template, or rename one through
`alterJobTemplate`, chooses where on the server's file system this happens. The
last path component of the deleted directory is always `job-<id>`.
### Error message and/or stacktrace
None. The requests succeed. When `<stagingDir>/<metalake>` doesn't exist
yet, the job fails with `Failed to start shell process ... output.log (No such
file or directory)`.
### How to reproduce
Verified on main by calling `JobManager` directly (`registerJobTemplate` →
`runJob` → `deleteJobTemplate`). The REST requests below are the equivalent,
with the default `gravitino.job.stagingDir=/tmp/gravitino/jobs/staging`:
1. Run any job in metalake `example`, so that
`/tmp/gravitino/jobs/staging/example` exists.
2. Register a template whose name escapes the staging directory:
```shell
curl -X POST -H "Accept: application/vnd.gravitino.v1+json" -H
"Content-Type: application/json" -d '{
"jobTemplate": {"name": "../../escape", "jobType": "shell",
"executable": "/bin/echo", "arguments": ["hello"]}
}' http://localhost:8090/api/metalakes/example/jobs/templates
```
3. Run a job from it:
```shell
curl -X POST -H "Accept: application/vnd.gravitino.v1+json" -H
"Content-Type: application/json" -d '{
"jobTemplateName": "../../escape", "jobConf": {}
}' http://localhost:8090/api/metalakes/example/jobs/runs
```
4. `/tmp/gravitino/jobs/escape/job-<id>/` now contains `echo`, `output.log`
and `error.log`, outside the staging directory.
5. Deleting the template, or the job expiring after
`gravitino.job.stagingDirKeepTimeInMs`, deletes
`/tmp/gravitino/jobs/escape/job-<id>`.
### Additional context
Found while working on multi-node job output retrieval (#12716), part of
epic #12667.
Possible fix:
- Validate job template names on register and rename.
- Add a containment check when building the staging path: normalize it, and
fail the run if it's outside `gravitino.job.stagingDir`.
Names containing `/` currently work (they create nested directories), so the
validation should decide whether to reject only traversal (`..` segments), or
all path separators.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
