diff --git a/availableVersions.json b/availableVersions.json index 880a3e5..a3e5279 100644 --- a/availableVersions.json +++ b/availableVersions.json @@ -1,6 +1,6 @@ { - "production": "1.4.6", - "test": "1.4.6", - "local": "1.4.6", - "all": "1.4.6" + "production": "1.4.7", + "test": "1.4.7", + "local": "1.4.7", + "all": "1.4.7" } diff --git a/src/api/handlers_more_test.go b/src/api/handlers_more_test.go index 95170c0..334acc7 100644 --- a/src/api/handlers_more_test.go +++ b/src/api/handlers_more_test.go @@ -425,6 +425,41 @@ func TestWriteToFileHandler(t *testing.T) { return ex, &fs } + // The name is joined onto the build dir. Anything but a plain file name + // would let a caller write wherever the agent can — as root, on most devices. + t.Run("refuses a file name that leaves the build dir", func(t *testing.T) { + root := t.TempDir() + buildDir := filepath.Join(root, "build") + require.NoError(t, os.Mkdir(buildDir, 0o755)) + + for _, fileName := range []string{ + "../escaped.tgz", + "../../escaped.tgz", + "sub/escaped.tgz", + filepath.Join(root, "escaped.tgz"), + "..", + ".", + "", + } { + ex, fs := newWriteEx(t, buildDir) + + res, err := ex.writeToFileHandler(context.Background(), messenger.Result{ + Details: systemDetails(), + Arguments: []interface{}{"BEGIN", fileName, "dev_100_myapp", uint64(4), "t1"}, + }) + + require.Error(t, err, fileName) + assert.Nil(t, res, fileName) + assert.Nil(t, fs.GetActiveTransfer("dev_100_myapp"), fileName) + } + + _, statErr := os.Stat(filepath.Join(root, "escaped.tgz")) + assert.True(t, os.IsNotExist(statErr), "nothing was written next to the build dir") + entries, readErr := os.ReadDir(buildDir) + require.NoError(t, readErr) + assert.Empty(t, entries) + }) + t.Run("writes a hex-decoded data chunk to disk", func(t *testing.T) { buildDir := t.TempDir() ex, fs := newWriteEx(t, buildDir) diff --git a/src/api/privilege_gates_test.go b/src/api/privilege_gates_test.go index 0159c20..0ddb755 100644 --- a/src/api/privilege_gates_test.go +++ b/src/api/privilege_gates_test.go @@ -8,11 +8,13 @@ import ( "io" "net" "os/exec" + "reagent/apps" "reagent/common" "reagent/container" "reagent/errdefs" "reagent/messenger" "reagent/messenger/topics" + "reagent/store" "reagent/terminal" "reagent/testutil/fakes" "reagent/testutil/mocks" @@ -361,19 +363,23 @@ func TestRequestTerminalSessHandlerRecordsTheOwner(t *testing.T) { } }) docker := mocks.NewContainer(t) - docker.EXPECT().ExecCommand(mock.Anything, "app_1_prod", []string{"cat", "/etc/shells"}).Return(container.HijackedResponse{ + docker.EXPECT().ExecCommand(mock.Anything, "dev_1_shellapp", []string{"cat", "/etc/shells"}).Return(container.HijackedResponse{ Conn: shells, Reader: bufio.NewReader(strings.NewReader("/bin/sh\n/bin/bash\n")), }, nil).Once() - docker.EXPECT().ExecAttach(mock.Anything, "app_1_prod", "/bin/bash").Return(container.HijackedResponse{ + docker.EXPECT().ExecAttach(mock.Anything, "dev_1_shellapp", "/bin/bash").Return(container.HijackedResponse{ Conn: attached, Reader: bufio.NewReader(attached), }, nil).Once() tm := terminal.NewTerminalManager(fakes.NewMessenger(), docker) _, m := grantPrivilege(true) - ex := &External{Privilege: newPrivilege(testConfig(), m), TerminalManager: &tm} - tc.result.Arguments = []interface{}{map[string]interface{}{"containerName": "app_1_prod"}} + ex := &External{ + Privilege: newPrivilege(testConfig(), m), + TerminalManager: &tm, + AppManager: appManagerWith(t, 1, "shellapp", common.DEV), + } + tc.result.Arguments = []interface{}{map[string]interface{}{"containerName": "dev_1_shellapp"}} res, err := wrapDetails(ex.requestTerminalSessHandler)(context.Background(), tc.result) @@ -387,6 +393,49 @@ func TestRequestTerminalSessHandlerRecordsTheOwner(t *testing.T) { } } +// appManagerWith is an AppManager whose store holds one installed app. +func appManagerWith(t *testing.T, appKey uint64, appName string, stage common.Stage) *apps.AppManager { + t.Helper() + db := newLogDB(t) + seedApp(t, db, appKey, appName, stage) + as := store.NewAppStore(db, fakes.NewMessenger()) + return &apps.AppManager{AppStore: &as} +} + +// DEVELOP on a device grants a shell in its apps' containers, never in any +// other container on the host — on an appliance, the platform's own database. +// The container mock is strict: a refused name that still reached Docker +// fails the test. +func TestRequestTerminalSessHandlerStaysWithinApps(t *testing.T) { + for _, name := range []string{ + "ironflock-postgres-1", // a platform container on an appliance + "prod_9_otherapp", // shaped like an app, but not installed + "dev_1_otherapp", // an installed app's key, another name + "prod_1_shellapp", // the installed app, but another stage + "/dev_1_shellapp", // not the exact name the agent gives it + "dev_1_shellapp_compose-web-1", // a container it did not start as the app + } { + t.Run(name, func(t *testing.T) { + tm := terminal.NewTerminalManager(fakes.NewMessenger(), mocks.NewContainer(t)) + ex := &External{ + Privilege: priv(t, true), + TerminalManager: &tm, + AppManager: appManagerWith(t, 1, "shellapp", common.DEV), + } + + res, err := ex.requestTerminalSessHandler(context.Background(), messenger.Result{ + Details: systemDetails(), + Arguments: []interface{}{map[string]interface{}{"containerName": name}}, + }) + + require.Error(t, err) + assert.Nil(t, res) + assert.True(t, errdefs.IsContainerNotFound(err)) + assert.Empty(t, tm.ActiveSessions) + }) + } +} + func TestGetCurrentIPAddressesPrivilegeGate(t *testing.T) { t.Run("denies a caller without READ", func(t *testing.T) { details, m := grantPrivilege(false) diff --git a/src/api/query_logs_test.go b/src/api/query_logs_test.go index 577c1ce..98e910c 100644 --- a/src/api/query_logs_test.go +++ b/src/api/query_logs_test.go @@ -48,9 +48,18 @@ func payloadOf(t *testing.T, res *messenger.InvokeResult) common.Dict { return payload } +// newAppLogsEx is a privileged External on a device where prod_1_logapp is +// installed — the only kind of container the log procedures may read. +func newAppLogsEx(t *testing.T) (*External, *mocks.Container) { + t.Helper() + ex, cont, db := newLogManagerEx(t, true) + seedApp(t, db, 1, "logapp", common.PROD) + return ex, cont +} + func TestQueryAppLogsHandler(t *testing.T) { t.Run("returns a windowed slice with its provenance", func(t *testing.T) { - ex, cont, _ := newLogManagerEx(t, true) + ex, cont := newAppLogsEx(t) expectLogs(cont, stampedBody(10)) res, err := ex.queryAppLogsHandler(context.Background(), messenger.Result{ @@ -84,7 +93,7 @@ func TestQueryAppLogsHandler(t *testing.T) { "float64": float64(3), "string": "3", } { - ex, cont, _ := newLogManagerEx(t, true) + ex, cont := newAppLogsEx(t) expectLogs(cont, stampedBody(10)) res, err := ex.queryAppLogsHandler(context.Background(), messenger.Result{ @@ -101,7 +110,7 @@ func TestQueryAppLogsHandler(t *testing.T) { }) t.Run("accepts a relative window", func(t *testing.T) { - ex, cont, _ := newLogManagerEx(t, true) + ex, cont := newAppLogsEx(t) expectLogs(cont, stampedBody(4)) res, err := ex.queryAppLogsHandler(context.Background(), messenger.Result{ @@ -167,6 +176,21 @@ func TestQueryAppLogsHandler(t *testing.T) { } }) + // On an appliance the agent's host also runs the platform. A READ privilege + // on the device reaches its apps' logs, never the platform's. + t.Run("answers a container that is no app here as not found, unread", func(t *testing.T) { + ex, _ := newAppLogsEx(t) + + res, err := ex.queryAppLogsHandler(context.Background(), messenger.Result{ + Details: systemDetails(), + Arguments: []interface{}{map[string]interface{}{"containerName": "ironflock-postgres-1"}}, + }) + + require.Error(t, err) + assert.Nil(t, res) + assert.True(t, errdefs.IsContainerNotFound(err)) + }) + t.Run("denies an unprivileged caller before reading anything", func(t *testing.T) { details, m := grantPrivilege(false) cont := mocks.NewContainer(t) diff --git a/src/api/terminal_management.go b/src/api/terminal_management.go index 2db3734..c0774d8 100644 --- a/src/api/terminal_management.go +++ b/src/api/terminal_management.go @@ -150,6 +150,12 @@ func (ex *External) requestTerminalSessHandler(ctx context.Context, response mes return nil, errors.New("failed to parse containerName") } + // A shell only in an app's own container: Docker would exec into any + // container on the host, the platform's own on an appliance. + if err := ex.AppManager.AppStore.CheckAppContainer(containerName); err != nil { + return nil, err + } + termSess, err := ex.TerminalManager.RequestTerminalSession(containerName, fmt.Sprint(response.Details["caller_authid"])) if err != nil { return nil, err diff --git a/src/api/write_file.go b/src/api/write_file.go index bda0a10..d4cf7e8 100644 --- a/src/api/write_file.go +++ b/src/api/write_file.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "path/filepath" "reagent/errdefs" "reagent/filesystem" "reagent/messenger" @@ -48,6 +49,13 @@ func (ex *External) writeToFileHandler(ctx context.Context, response messenger.R return nil, errors.New("failed to parse containerName argument") } + // The file lands directly in the build directory. Joined onto it unchecked, + // a "../" in the caller's name wrote anywhere the agent can — as root, on + // most devices. + if fileName != filepath.Base(fileName) || !filepath.IsLocal(fileName) { + return nil, fmt.Errorf("invalid file name %q: must be a plain file name", fileName) + } + total, ok := totalArg.(uint64) if !ok { return nil, errors.New("failed to parse total argument") diff --git a/src/logging/applog.go b/src/logging/applog.go index fe72e3b..f07722d 100644 --- a/src/logging/applog.go +++ b/src/logging/applog.go @@ -755,6 +755,13 @@ func (lm *LogManager) GetLogHistory(containerName string) ([]string, error) { stringLogEntries := logEntriesToString(history) + // Docker is only asked about an app's own container. Anything else keeps + // the empty answer it always got for an unknown app: a console that opens + // before the agent knows the app must still go on to subscribe. + if lm.AppStore.CheckAppContainer(containerName) != nil { + return stringLogEntries, nil + } + if containsOnlyAgentLogs || len(history) == 0 { ctx := context.Background() options := common.Dict{"follow": false, "stdout": true, "stderr": true, "tail": "50"} diff --git a/src/logging/query.go b/src/logging/query.go index 8b53f90..c663257 100644 --- a/src/logging/query.go +++ b/src/logging/query.go @@ -84,8 +84,8 @@ type LogQueryResult struct { // QueryLogs reads a bounded, optionally time-windowed slice of a container's logs. func (lm *LogManager) QueryLogs(ctx context.Context, request LogQueryRequest) (LogQueryResult, error) { - if strings.TrimSpace(request.ContainerName) == "" { - return LogQueryResult{}, errors.New("container name is empty") + if err := lm.AppStore.CheckAppContainer(request.ContainerName); err != nil { + return LogQueryResult{}, err } tail := request.Tail diff --git a/src/logging/query_test.go b/src/logging/query_test.go index d927e43..d961ae1 100644 --- a/src/logging/query_test.go +++ b/src/logging/query_test.go @@ -31,6 +31,15 @@ func stampedLines(count int) string { return strings.Join(lines, "\n") + "\n" } +// newAppManager is a LogManager on a device where prod_1_logapp is installed — +// the only kind of container a log read may name. +func newAppManager(t *testing.T) (*LogManager, *mocks.Container) { + t.Helper() + lm, cont, _, db := newTestManager(t) + addApp(t, db, 1, "logapp", common.PROD) + return lm, cont +} + func reader(body string) io.ReadCloser { return io.NopCloser(strings.NewReader(body)) } @@ -50,7 +59,7 @@ func alwaysReturn(cont *mocks.Container, containerName string, body string) { func TestQueryLogsTailAndOffset(t *testing.T) { t.Run("returns the newest tail and reports the trim", func(t *testing.T) { - lm, cont, _, _ := newTestManager(t) + lm, cont := newAppManager(t) alwaysReturn(cont, "prod_1_logapp", stampedLines(10)) result, err := lm.QueryLogs(context.Background(), LogQueryRequest{ @@ -69,7 +78,7 @@ func TestQueryLogsTailAndOffset(t *testing.T) { // Docker has no offset, so paging back means over-reading and discarding the // newest lines here. Ten lines, drop the newest two, then keep three: 5,6,7. t.Run("offset pages further back", func(t *testing.T) { - lm, cont, _, _ := newTestManager(t) + lm, cont := newAppManager(t) alwaysReturn(cont, "prod_1_logapp", stampedLines(10)) result, err := lm.QueryLogs(context.Background(), LogQueryRequest{ @@ -85,7 +94,7 @@ func TestQueryLogsTailAndOffset(t *testing.T) { }) t.Run("an offset past the whole log yields nothing rather than the newest lines", func(t *testing.T) { - lm, cont, _, _ := newTestManager(t) + lm, cont := newAppManager(t) alwaysReturn(cont, "prod_1_logapp", stampedLines(5)) result, err := lm.QueryLogs(context.Background(), LogQueryRequest{ @@ -101,7 +110,7 @@ func TestQueryLogsTailAndOffset(t *testing.T) { // The read itself is bounded, not just the answer: asking to page back a // million lines must not pull a million lines off the daemon. t.Run("bounds the read, not only the result", func(t *testing.T) { - lm, cont, _, _ := newTestManager(t) + lm, cont := newAppManager(t) // Capture the FIRST call only — the second is the unbounded retention // probe, which legitimately asks for everything. @@ -127,7 +136,7 @@ func TestQueryLogsTailAndOffset(t *testing.T) { func TestQueryLogsWindow(t *testing.T) { t.Run("passes the window through as absolute bounds and asks for timestamps", func(t *testing.T) { - lm, cont, _, _ := newTestManager(t) + lm, cont := newAppManager(t) // Only the FIRST call is the window; the second is the unbounded // retention probe and deliberately carries no bounds. @@ -158,7 +167,7 @@ func TestQueryLogsWindow(t *testing.T) { // closed after one line — that is what separates "the device was quiet" from // "your window is older than what survived rotation". t.Run("reports the oldest line the device can still serve", func(t *testing.T) { - lm, cont, _, _ := newTestManager(t) + lm, cont := newAppManager(t) alwaysReturn(cont, "prod_1_logapp", stampedLines(4)) result, err := lm.QueryLogs(context.Background(), LogQueryRequest{ContainerName: "prod_1_logapp"}) @@ -192,7 +201,7 @@ func TestQueryLogsFallback(t *testing.T) { }) t.Run("surfaces a real docker failure instead of quietly serving history", func(t *testing.T) { - lm, cont, _, _ := newTestManager(t) + lm, cont := newAppManager(t) cont.EXPECT().Logs(mock.Anything, "prod_1_logapp", mock.Anything). Return(nil, errors.New("permission denied on the docker socket")) @@ -210,6 +219,59 @@ func TestQueryLogsFallback(t *testing.T) { }) } +// The container name comes from the caller, and Docker answers for any +// container on the host — on an appliance, the platform's own as well. A read +// reaches only the containers of apps installed on this device. The container +// mock is strict, so a refused name that still reached Docker fails the test. +func TestLogReadsStayWithinApps(t *testing.T) { + foreign := []string{ + "ironflock-postgres-1", // a platform container on an appliance + "prod_9_otherapp", // shaped like an app, but not installed + "prod_1_otherapp", // an installed app's key, another name + "dev_1_logapp", // the installed app, but another stage + "pub_1_logapp", // a release build of a PROD-only app + "/prod_1_logapp", // not the exact name the agent gives it + "prod_1_logapp_compose-web-1", // one service of it, not the app + } + + t.Run("a windowed read refuses any other container as not found", func(t *testing.T) { + for _, name := range foreign { + lm, _ := newAppManager(t) + + _, err := lm.QueryLogs(context.Background(), LogQueryRequest{ContainerName: name}) + + assert.True(t, errdefs.IsContainerNotFound(err), "%s: %v", name, err) + } + }) + + // A console that opens before the agent knows the app must still go on to + // subscribe, so an app-shaped name keeps getting the empty list an unknown + // app always got. A name that does not even parse was, and is, an error. + t.Run("the history read answers any other container with nothing", func(t *testing.T) { + for _, name := range foreign { + lm, _ := newAppManager(t) + + history, err := lm.GetLogHistory(name) + + if _, _, _, parseErr := common.ParseContainerName(name); parseErr == nil { + require.NoError(t, err, name) + } + assert.Empty(t, history, name) + } + }) + + t.Run("a DEV app's release build is its own", func(t *testing.T) { + lm, cont, _, db := newTestManager(t) + addApp(t, db, 1, "logapp", common.DEV) + alwaysReturn(cont, "pub_1_logapp", stampedLines(2)) + + result, err := lm.QueryLogs(context.Background(), LogQueryRequest{ContainerName: "pub_1_logapp"}) + + require.NoError(t, err) + assert.Len(t, result.Lines, 2) + }) +} + // composeContainer is one container of the test app's compose project, as the // Docker API lists it: named --1, with a leading slash. func composeContainer(service string) container.ContainerResult { @@ -240,7 +302,7 @@ func TestQueryLogsCompose(t *testing.T) { const db = "2026-08-04T09:00:00Z db starting\n2026-08-04T09:00:02Z db ready\n" t.Run("merges every service into one timeline", func(t *testing.T) { - lm, cont, _, _ := newTestManager(t) + lm, cont := newAppManager(t) composeProject(cont, composeContainer("web"), composeContainer("db")) alwaysReturn(cont, "web-id", web) alwaysReturn(cont, "db-id", db) @@ -262,7 +324,7 @@ func TestQueryLogsCompose(t *testing.T) { // Docker applies the tail per container, so each is over-read; the tail // that matters is the app's. t.Run("keeps the newest lines of the app, not of each service", func(t *testing.T) { - lm, cont, _, _ := newTestManager(t) + lm, cont := newAppManager(t) composeProject(cont, composeContainer("web"), composeContainer("db")) alwaysReturn(cont, "web-id", web) alwaysReturn(cont, "db-id", db) @@ -277,7 +339,7 @@ func TestQueryLogsCompose(t *testing.T) { }) t.Run("drops the stamps the merge needed when none were asked for", func(t *testing.T) { - lm, cont, _, _ := newTestManager(t) + lm, cont := newAppManager(t) composeProject(cont, composeContainer("web"), composeContainer("db")) var asked []common.Dict @@ -311,7 +373,7 @@ func TestQueryLogsCompose(t *testing.T) { // Docker frames its stream — and a frame can hold several lines, or end in // the middle of one. t.Run("demultiplexes a service without a TTY", func(t *testing.T) { - lm, cont, _, _ := newTestManager(t) + lm, cont := newAppManager(t) composeProject(cont, composeContainer("worker")) var framed bytes.Buffer @@ -334,7 +396,7 @@ func TestQueryLogsCompose(t *testing.T) { }) t.Run("names a service with its own container_name as compose does", func(t *testing.T) { - lm, cont, _, _ := newTestManager(t) + lm, cont := newAppManager(t) broker := composeContainer("broker") broker.Names = []string{"/my-broker"} composeProject(cont, broker) @@ -347,7 +409,7 @@ func TestQueryLogsCompose(t *testing.T) { }) t.Run("skips a container removed between listing and read", func(t *testing.T) { - lm, cont, _, _ := newTestManager(t) + lm, cont := newAppManager(t) composeProject(cont, composeContainer("web"), composeContainer("db")) cont.EXPECT().Logs(mock.Anything, "web-id", mock.Anything). Return(nil, errdefs.ContainerNotFound(errors.New("No such container"))) @@ -363,7 +425,7 @@ func TestQueryLogsCompose(t *testing.T) { // The containers are there; serving the (usually empty) stored history // would present a failed read as an app that said nothing. t.Run("surfaces a failed read instead of serving history", func(t *testing.T) { - lm, cont, _, _ := newTestManager(t) + lm, cont := newAppManager(t) composeProject(cont, composeContainer("web")) cont.EXPECT().Logs(mock.Anything, "web-id", mock.Anything). Return(nil, errors.New("permission denied on the docker socket")) @@ -405,7 +467,7 @@ func TestQueryLogsTimestampToggle(t *testing.T) { // The console needs them off; a diagnosis needs them on. Either way the // retention floor still comes from the separate probe. t.Run("timestamps can be turned off for the window", func(t *testing.T) { - lm, cont, _, _ := newTestManager(t) + lm, cont := newAppManager(t) var seen common.Dict cont.EXPECT().Logs(mock.Anything, "prod_1_logapp", mock.Anything). diff --git a/src/release/version.txt b/src/release/version.txt index 7b5753f..b000a6a 100644 --- a/src/release/version.txt +++ b/src/release/version.txt @@ -1 +1 @@ -1.4.6 \ No newline at end of file +1.4.7 \ No newline at end of file diff --git a/src/store/app_store.go b/src/store/app_store.go index cce7f53..3d57140 100644 --- a/src/store/app_store.go +++ b/src/store/app_store.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "reagent/common" + "reagent/errdefs" "reagent/messenger" "reagent/messenger/topics" "reagent/persistence" @@ -69,6 +70,43 @@ func (am *AppStore) GetApp(appKey uint64, stage common.Stage) (*common.App, erro return app, nil } +// CheckAppContainer refuses a container name that does not belong to an app +// installed on this device. +// +// Procedures that act on a container — its logs, a shell in it — take the name +// from their caller, and Docker answers for any container on the host. On an +// appliance that host also runs the platform itself — its database, router, +// registry — which no privilege on the device may reach. Any other name is +// answered "not found", so the answer does not tell a caller which other +// containers exist either. +func (am *AppStore) CheckAppContainer(containerName string) error { + notFound := errdefs.ContainerNotFound(fmt.Errorf("no app container %q on this device", containerName)) + + stage, appKey, _, err := common.ParseContainerName(containerName) + if err != nil || stage == "" { + return notFound + } + + app, err := am.GetApp(appKey, stage) + if err != nil { + return err + } + if app == nil { + return notFound + } + + if containerName == common.BuildContainerName(stage, appKey, app.AppName) { + return nil + } + + // A DEV app's release build logs under pub_, which parses to DEV. + if stage == common.DEV && containerName == common.BuildContainerName("pub", appKey, app.AppName) { + return nil + } + + return notFound +} + func (am *AppStore) AddApp(payload common.TransitionPayload) (*common.App, error) { app := &common.App{ AppKey: payload.AppKey,