review: refuse a process named ".", ".." or with a leading dash, so removing one cannot delete the mesh's own directory (hq ADR 0118)
removeProcess deletes filepath.Join(daemonRoot, name) whole; a process named ".." made that /var/lib/mesh. The declaration and the removal now hold the name to one rule.
This commit is contained in:
@@ -303,10 +303,11 @@ func unitValue(key, value string) string {
|
||||
// have all vanished is forgotten rather than reported as removed.
|
||||
func removeProcess(ctx context.Context, a store.Applied, run Runner) (string, string, error) {
|
||||
name := a.Target
|
||||
if name == "" || strings.ContainsAny(name, "/ \t") {
|
||||
// The name is a unit name and a directory under the mesh's own. One that could climb out
|
||||
// of either is refused rather than acted on, whatever wrote it into the record.
|
||||
return "", "", fmt.Errorf("a process recorded under %q cannot be removed by name", name)
|
||||
if problem := declaration.ProcessNameProblem(name); problem != "" {
|
||||
// The name is a unit name and a directory under the mesh's own, and what goes is that
|
||||
// directory, whole. One that could climb out of either — ".." is the mesh's own directory's
|
||||
// parent — is refused rather than acted on, whatever wrote it into the record.
|
||||
return "", "", fmt.Errorf("a process recorded under %q cannot be removed by name: %s", name, problem)
|
||||
}
|
||||
found := false
|
||||
for _, unit := range []string{name + ".timer", name + ".service"} {
|
||||
|
||||
@@ -224,3 +224,35 @@ func TestAnUndeclaredProcessIsRemovedWithItsUnitAndBundle(t *testing.T) {
|
||||
t.Errorf("removing a process that is already gone failed: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAProcessRecordedUnderAPathlikeNameIsRefusedNotRemoved(t *testing.T) {
|
||||
// filepath.Join(daemonRoot, "..") is the mesh's own directory, and removal deletes what that
|
||||
// names, whole. A record is what some host wrote, perhaps under looser rules than today's, so
|
||||
// the removal holds the name to the declaration's rule again (novox/hq ADR 0118).
|
||||
root := t.TempDir()
|
||||
bundles := filepath.Join(root, "daemons")
|
||||
wasUnits, wasBundles := unitDir, daemonRoot
|
||||
unitDir, daemonRoot = t.TempDir(), bundles
|
||||
t.Cleanup(func() { unitDir, daemonRoot = wasUnits, wasBundles })
|
||||
precious := filepath.Join(root, "state.json")
|
||||
if err := os.MkdirAll(filepath.Join(bundles, "other"), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(precious, []byte("{}"), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
for _, name := range []string{"..", ".", "", "-x"} {
|
||||
known := store.State{Resources: []store.Applied{{ID: "p", Type: "process", Target: name}}}
|
||||
d := parse(t, `{"declaration":1,"resources":[
|
||||
{"id":"f","type":"file","path":"`+filepath.Join(t.TempDir(), "a")+`","content":"a\n"}
|
||||
]}`)
|
||||
if _, _, err := Apply(context.Background(), archHost(t), d, known, store.OriginCarried, noServices, nil, nil); err == nil {
|
||||
t.Errorf("a process recorded as %q was removed by name", name)
|
||||
}
|
||||
for _, still := range []string{precious, filepath.Join(bundles, "other")} {
|
||||
if _, err := os.Stat(still); err != nil {
|
||||
t.Fatalf("removing a process recorded as %q took %s with it", name, still)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user