Skip to content

Commit 292e7a4

Browse files
authored
Merge pull request #9383 from hashicorp/b-template-escape
client: fix interpolation in template source
2 parents 5b72385 + 252ac1e commit 292e7a4

File tree

5 files changed

+410
-2
lines changed

5 files changed

+410
-2
lines changed

client/allocrunner/taskrunner/template/template.go

+21-2
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package template
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"math/rand"
78
"os"
@@ -17,6 +18,7 @@ import (
1718
"github.com/hashicorp/consul-template/signals"
1819
envparse "github.com/hashicorp/go-envparse"
1920
multierror "github.com/hashicorp/go-multierror"
21+
"github.com/hashicorp/nomad/client/allocdir"
2022
"github.com/hashicorp/nomad/client/allocrunner/taskrunner/interfaces"
2123
"github.com/hashicorp/nomad/client/config"
2224
"github.com/hashicorp/nomad/client/taskenv"
@@ -38,6 +40,11 @@ const (
3840
DefaultMaxTemplateEventRate = 3 * time.Second
3941
)
4042

43+
var (
44+
sourceEscapesErr = errors.New("template source path escapes alloc directory")
45+
destEscapesErr = errors.New("template destination path escapes alloc directory")
46+
)
47+
4148
// TaskTemplateManager is used to run a set of templates for a given task
4249
type TaskTemplateManager struct {
4350
// config holds the template managers configuration
@@ -548,6 +555,18 @@ func parseTemplateConfigs(config *TaskTemplateManagerConfig) (map[*ctconf.Templa
548555
sandboxEnabled := !config.ClientConfig.TemplateConfig.DisableSandbox
549556
taskEnv := config.EnvBuilder.Build()
550557

558+
// Make NOMAD_{ALLOC,TASK,SECRETS}_DIR relative paths to avoid treating
559+
// them as sandbox escapes when using containers.
560+
if taskEnv.EnvMap[taskenv.AllocDir] == allocdir.SharedAllocContainerPath {
561+
taskEnv.EnvMap[taskenv.AllocDir] = allocdir.SharedAllocName
562+
}
563+
if taskEnv.EnvMap[taskenv.TaskLocalDir] == allocdir.TaskLocalContainerPath {
564+
taskEnv.EnvMap[taskenv.TaskLocalDir] = allocdir.TaskLocal
565+
}
566+
if taskEnv.EnvMap[taskenv.SecretsDir] == allocdir.TaskSecretsContainerPath {
567+
taskEnv.EnvMap[taskenv.SecretsDir] = allocdir.TaskSecrets
568+
}
569+
551570
ctmpls := make(map[*ctconf.TemplateConfig]*structs.Template, len(config.Templates))
552571
for _, tmpl := range config.Templates {
553572
var src, dest string
@@ -560,7 +579,7 @@ func parseTemplateConfigs(config *TaskTemplateManagerConfig) (map[*ctconf.Templa
560579
}
561580
escapes := helper.PathEscapesSandbox(config.TaskDir, src)
562581
if escapes && sandboxEnabled {
563-
return nil, fmt.Errorf("template source path escapes alloc directory")
582+
return nil, sourceEscapesErr
564583
}
565584
}
566585

@@ -572,7 +591,7 @@ func parseTemplateConfigs(config *TaskTemplateManagerConfig) (map[*ctconf.Templa
572591
dest = filepath.Join(config.TaskDir, dest)
573592
escapes := helper.PathEscapesSandbox(config.TaskDir, dest)
574593
if escapes && sandboxEnabled {
575-
return nil, fmt.Errorf("template destination path escapes alloc directory")
594+
return nil, destEscapesErr
576595
}
577596
}
578597

client/allocrunner/taskrunner/template/template_test.go

+251
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,11 @@ import (
1717
"time"
1818

1919
ctestutil "github.com/hashicorp/consul/sdk/testutil"
20+
"github.com/hashicorp/nomad/client/allocdir"
2021
"github.com/hashicorp/nomad/client/config"
2122
"github.com/hashicorp/nomad/client/taskenv"
2223
"github.com/hashicorp/nomad/helper"
24+
"github.com/hashicorp/nomad/helper/testlog"
2325
"github.com/hashicorp/nomad/helper/uuid"
2426
"github.com/hashicorp/nomad/nomad/mock"
2527
"github.com/hashicorp/nomad/nomad/structs"
@@ -1452,6 +1454,255 @@ func TestTaskTemplateManager_Config_VaultNamespace_TaskOverride(t *testing.T) {
14521454
assert.Equal(overriddenNS, *ctconf.Vault.Namespace, "Vault Namespace Value")
14531455
}
14541456

1457+
// TestTaskTemplateManager_Escapes asserts that when sandboxing is enabled
1458+
// interpolated paths are not incorrectly treated as escaping the alloc dir.
1459+
func TestTaskTemplateManager_Escapes(t *testing.T) {
1460+
t.Parallel()
1461+
1462+
clientConf := config.DefaultConfig()
1463+
require.False(t, clientConf.TemplateConfig.DisableSandbox, "expected sandbox to be disabled")
1464+
1465+
// Set a fake alloc dir to make test output more realistic
1466+
clientConf.AllocDir = "/fake/allocdir"
1467+
1468+
clientConf.Node = mock.Node()
1469+
alloc := mock.Alloc()
1470+
task := alloc.Job.TaskGroups[0].Tasks[0]
1471+
logger := testlog.HCLogger(t)
1472+
allocDir := allocdir.NewAllocDir(logger, filepath.Join(clientConf.AllocDir, alloc.ID))
1473+
taskDir := allocDir.NewTaskDir(task.Name)
1474+
1475+
containerEnv := func() *taskenv.Builder {
1476+
// To emulate a Docker or exec tasks we must copy the
1477+
// Set{Alloc,Task,Secrets}Dir logic in taskrunner/task_dir_hook.go
1478+
b := taskenv.NewBuilder(clientConf.Node, alloc, task, clientConf.Region)
1479+
b.SetAllocDir(allocdir.SharedAllocContainerPath)
1480+
b.SetTaskLocalDir(allocdir.TaskLocalContainerPath)
1481+
b.SetSecretsDir(allocdir.TaskSecretsContainerPath)
1482+
return b
1483+
}
1484+
1485+
rawExecEnv := func() *taskenv.Builder {
1486+
// To emulate a unisolated tasks we must copy the
1487+
// Set{Alloc,Task,Secrets}Dir logic in taskrunner/task_dir_hook.go
1488+
b := taskenv.NewBuilder(clientConf.Node, alloc, task, clientConf.Region)
1489+
b.SetAllocDir(taskDir.SharedAllocDir)
1490+
b.SetTaskLocalDir(taskDir.LocalDir)
1491+
b.SetSecretsDir(taskDir.SecretsDir)
1492+
return b
1493+
}
1494+
1495+
cases := []struct {
1496+
Name string
1497+
Config func() *TaskTemplateManagerConfig
1498+
1499+
// Set to skip a test; remove once bugs are fixed
1500+
Skip bool
1501+
1502+
// Expected paths to be returned if Err is nil
1503+
SourcePath string
1504+
DestPath string
1505+
1506+
// Err is the expected error to be returned or nil
1507+
Err error
1508+
}{
1509+
{
1510+
Name: "ContainerOk",
1511+
Config: func() *TaskTemplateManagerConfig {
1512+
return &TaskTemplateManagerConfig{
1513+
ClientConfig: clientConf,
1514+
TaskDir: taskDir.Dir,
1515+
EnvBuilder: containerEnv(),
1516+
Templates: []*structs.Template{
1517+
{
1518+
SourcePath: "${NOMAD_TASK_DIR}/src",
1519+
DestPath: "${NOMAD_SECRETS_DIR}/dst",
1520+
},
1521+
},
1522+
}
1523+
},
1524+
SourcePath: filepath.Join(taskDir.Dir, "local/src"),
1525+
DestPath: filepath.Join(taskDir.Dir, "secrets/dst"),
1526+
},
1527+
{
1528+
Name: "ContainerSrcEscapesErr",
1529+
Config: func() *TaskTemplateManagerConfig {
1530+
return &TaskTemplateManagerConfig{
1531+
ClientConfig: clientConf,
1532+
TaskDir: taskDir.Dir,
1533+
EnvBuilder: containerEnv(),
1534+
Templates: []*structs.Template{
1535+
{
1536+
SourcePath: "/etc/src_escapes",
1537+
DestPath: "${NOMAD_SECRETS_DIR}/dst",
1538+
},
1539+
},
1540+
}
1541+
},
1542+
Err: sourceEscapesErr,
1543+
},
1544+
{
1545+
Name: "ContainerSrcEscapesOk",
1546+
Config: func() *TaskTemplateManagerConfig {
1547+
unsafeConf := clientConf.Copy()
1548+
unsafeConf.TemplateConfig.DisableSandbox = true
1549+
return &TaskTemplateManagerConfig{
1550+
ClientConfig: unsafeConf,
1551+
TaskDir: taskDir.Dir,
1552+
EnvBuilder: containerEnv(),
1553+
Templates: []*structs.Template{
1554+
{
1555+
SourcePath: "/etc/src_escapes_ok",
1556+
DestPath: "${NOMAD_SECRETS_DIR}/dst",
1557+
},
1558+
},
1559+
}
1560+
},
1561+
SourcePath: "/etc/src_escapes_ok",
1562+
DestPath: filepath.Join(taskDir.Dir, "secrets/dst"),
1563+
},
1564+
{
1565+
Name: "ContainerDstAbsoluteOk",
1566+
Config: func() *TaskTemplateManagerConfig {
1567+
return &TaskTemplateManagerConfig{
1568+
ClientConfig: clientConf,
1569+
TaskDir: taskDir.Dir,
1570+
EnvBuilder: containerEnv(),
1571+
Templates: []*structs.Template{
1572+
{
1573+
SourcePath: "${NOMAD_TASK_DIR}/src",
1574+
DestPath: "/etc/absolutely_relative",
1575+
},
1576+
},
1577+
}
1578+
},
1579+
SourcePath: filepath.Join(taskDir.Dir, "local/src"),
1580+
DestPath: filepath.Join(taskDir.Dir, "etc/absolutely_relative"),
1581+
},
1582+
{
1583+
Name: "ContainerDstAbsoluteEscapesErr",
1584+
Config: func() *TaskTemplateManagerConfig {
1585+
return &TaskTemplateManagerConfig{
1586+
ClientConfig: clientConf,
1587+
TaskDir: taskDir.Dir,
1588+
EnvBuilder: containerEnv(),
1589+
Templates: []*structs.Template{
1590+
{
1591+
SourcePath: "${NOMAD_TASK_DIR}/src",
1592+
DestPath: "../escapes",
1593+
},
1594+
},
1595+
}
1596+
},
1597+
Err: destEscapesErr,
1598+
},
1599+
{
1600+
Name: "ContainerDstAbsoluteEscapesOk",
1601+
Config: func() *TaskTemplateManagerConfig {
1602+
unsafeConf := clientConf.Copy()
1603+
unsafeConf.TemplateConfig.DisableSandbox = true
1604+
return &TaskTemplateManagerConfig{
1605+
ClientConfig: unsafeConf,
1606+
TaskDir: taskDir.Dir,
1607+
EnvBuilder: containerEnv(),
1608+
Templates: []*structs.Template{
1609+
{
1610+
SourcePath: "${NOMAD_TASK_DIR}/src",
1611+
DestPath: "../escapes",
1612+
},
1613+
},
1614+
}
1615+
},
1616+
SourcePath: filepath.Join(taskDir.Dir, "local/src"),
1617+
DestPath: filepath.Join(taskDir.Dir, "..", "escapes"),
1618+
},
1619+
//TODO: Fix this test. I *think* it should pass. The double
1620+
// joining of the task dir onto the destination seems like
1621+
// a bug. https://github.com/hashicorp/nomad/issues/9389
1622+
{
1623+
Skip: true,
1624+
Name: "RawExecOk",
1625+
Config: func() *TaskTemplateManagerConfig {
1626+
return &TaskTemplateManagerConfig{
1627+
ClientConfig: clientConf,
1628+
TaskDir: taskDir.Dir,
1629+
EnvBuilder: rawExecEnv(),
1630+
Templates: []*structs.Template{
1631+
{
1632+
SourcePath: "${NOMAD_TASK_DIR}/src",
1633+
DestPath: "${NOMAD_SECRETS_DIR}/dst",
1634+
},
1635+
},
1636+
}
1637+
},
1638+
SourcePath: filepath.Join(taskDir.Dir, "local/src"),
1639+
DestPath: filepath.Join(taskDir.Dir, "secrets/dst"),
1640+
},
1641+
{
1642+
Name: "RawExecSrcEscapesErr",
1643+
Config: func() *TaskTemplateManagerConfig {
1644+
return &TaskTemplateManagerConfig{
1645+
ClientConfig: clientConf,
1646+
TaskDir: taskDir.Dir,
1647+
EnvBuilder: rawExecEnv(),
1648+
Templates: []*structs.Template{
1649+
{
1650+
SourcePath: "/etc/src_escapes",
1651+
DestPath: "${NOMAD_SECRETS_DIR}/dst",
1652+
},
1653+
},
1654+
}
1655+
},
1656+
Err: sourceEscapesErr,
1657+
},
1658+
{
1659+
Name: "RawExecDstAbsoluteOk",
1660+
Config: func() *TaskTemplateManagerConfig {
1661+
return &TaskTemplateManagerConfig{
1662+
ClientConfig: clientConf,
1663+
TaskDir: taskDir.Dir,
1664+
EnvBuilder: rawExecEnv(),
1665+
Templates: []*structs.Template{
1666+
{
1667+
SourcePath: "${NOMAD_TASK_DIR}/src",
1668+
DestPath: "/etc/absolutely_relative",
1669+
},
1670+
},
1671+
}
1672+
},
1673+
SourcePath: filepath.Join(taskDir.Dir, "local/src"),
1674+
DestPath: filepath.Join(taskDir.Dir, "etc/absolutely_relative"),
1675+
},
1676+
}
1677+
1678+
for i := range cases {
1679+
tc := cases[i]
1680+
t.Run(tc.Name, func(t *testing.T) {
1681+
if tc.Skip {
1682+
t.Skip("FIXME: Skipping broken test")
1683+
}
1684+
config := tc.Config()
1685+
mapping, err := parseTemplateConfigs(config)
1686+
if tc.Err == nil {
1687+
// Ok path
1688+
require.NoError(t, err)
1689+
require.NotNil(t, mapping)
1690+
require.Len(t, mapping, 1)
1691+
for k := range mapping {
1692+
require.Equal(t, tc.SourcePath, *k.Source)
1693+
require.Equal(t, tc.DestPath, *k.Destination)
1694+
t.Logf("Rendering %s => %s", *k.Source, *k.Destination)
1695+
}
1696+
} else {
1697+
// Err path
1698+
assert.EqualError(t, err, tc.Err.Error())
1699+
require.Nil(t, mapping)
1700+
}
1701+
1702+
})
1703+
}
1704+
}
1705+
14551706
func TestTaskTemplateManager_BlockedEvents(t *testing.T) {
14561707
// The tests sets a template that need keys 0, 1, 2, 3, 4,
14571708
// then subsequently sets 0, 1, 2 keys

0 commit comments

Comments
 (0)