Workshopctl instance context

internal/daemon/api_workshopctl.go

@@ -20,7 +20,10 @@ import (
 
 	"github.com/jessevdk/go-flags"
 
+	"github.com/canonical/workshop/internal/logger"
+	"github.com/canonical/workshop/internal/overlord/hookstate"
 	"github.com/canonical/workshop/internal/overlord/hookstate/ctlcmd"
+	"github.com/canonical/workshop/internal/workshop"
 )
 
 // workshopCtlOptions holds the various options with which workshopctl is invoked.
@@ -60,17 +63,18 @@ func v1PostWorkshopCtl(c *Command, r *http.Request, _ *userState) Response {
 		return statusForbidden("cannot get remote user: %w", err)
 	}
 
-	// Ignore missing context error to allow 'workshopctl -h' without a context;
-	// Actual context is validated later by get/set.
-	context, _ := c.d.overlord.HookManager().Context(reqData.ContextID)
+	hookContext, response := workshopctlHookContext(c, r, reqData.ContextID)
+	if response != nil {
+		return response
+	}
 
 	if reqData.Stdin != nil {
-		context.Lock()
-		context.Set("stdin", reqData.Stdin)
-		context.Unlock()
+		hookContext.Lock()
+		hookContext.Set("stdin", reqData.Stdin)
+		hookContext.Unlock()
 	}
 
-	stdout, stderr, err := ctlcmd.Run(context, reqData.Args, uid)
+	stdout, stderr, err := ctlcmd.Run(r.Context(), hookContext, reqData.Args, uid)
 	if err != nil {
 		if e, ok := err.(*flags.Error); ok && e.Type == flags.ErrHelp {
 			stdout = []byte(e.Error())
@@ -86,3 +90,64 @@ func v1PostWorkshopCtl(c *Command, r *http.Request, _ *userState) Response {
 
 	return SyncResponse(result, http.StatusOK)
 }
+
+// workshopctlHookContext returns the hook context used to execute a
+// workshopctl command. A supplied cookie selects an existing context;
+// otherwise the workshop instance ID is validated before creating an
+// ephemeral context.
+func workshopctlHookContext(
+	c *Command,
+	r *http.Request,
+	contextID string,
+) (*hookstate.Context, Response) {
+	if contextID != "" {
+		return workshopctlHookContextFromContextID(c, contextID)
+	}
+	return workshopctlHookContextFromInstanceID(c, r)
+}
+
+// workshopctlHookContextFromContextID returns the active hook context
+// identified by the request's context ID, supplied to hooks as WORKSHOP_COOKIE.
+// An invalid context ID is rejected rather than falling back to instance ID
+// authentication.
+func workshopctlHookContextFromContextID(
+	c *Command,
+	contextID string,
+) (*hookstate.Context, Response) {
+	hookContext, err := c.d.overlord.HookManager().Context(contextID)
+	if err != nil {
+		return nil, statusBadRequest("invalid workshop cookie")
+	}
+	return hookContext, nil
+}
+
+// workshopctlHookContextFromInstanceID validates that the requesting user owns
+// the workshop instance identified by the request context, then creates a
+// taskless context for this individual workshopctl invocation.
+func workshopctlHookContextFromInstanceID(
+	c *Command,
+	r *http.Request,
+) (*hookstate.Context, Response) {
+	instanceID, _ := r.Context().
+		Value(workshop.ContextWorkshopInstanceID).(string)
+	if instanceID == "" {
+		return nil, statusBadRequest("workshop instance ID not supplied")
+	}
+
+	valid, err := c.d.overlord.WorkshopManager().
+		OwnsWorkshopInstanceID(r.Context(), instanceID)
+	if err != nil {
+		logger.Noticef("cannot validate workshop instance ID: %v", err)
+		return nil, statusInternalError("internal error occurred validating workshop instance id")
+	}
+	if !valid {
+		return nil, statusForbidden("invalid workshop instance ID")
+	}
+
+	hookContext, err := c.d.overlord.HookManager().NewEphemeralContext()
+	if err != nil {
+		logger.Noticef("cannot create workshop context: %v", err)
+		return nil, statusInternalError("internal error occurred")
+	}
+	return hookContext, nil
+}

internal/daemon/api_workshopctl_test.go

@@ -16,21 +16,48 @@ package daemon
 
 import (
 	"bytes"
+	"context"
 	"net/http"
 
 	"gopkg.in/check.v1"
+
+	"github.com/canonical/workshop/internal/workshop"
+	"github.com/canonical/workshop/internal/workshop/fakebackend"
 )
 
-func (s *apiSuite) TestWorkshopHelpCtlNoContext(c *check.C) {
+func (s *apiSuite) addWorkshopWithInstanceID(instanceID string) {
+	s.b.Workshops[s.project.ProjectId] = map[string]*fakebackend.FakeWorkshop{
+		"test-workshop": {
+			Workshop: &workshop.Workshop{
+				Name:       "test-workshop",
+				Project:    s.project,
+				InstanceID: instanceID,
+			},
+		},
+	}
+}
+
+// TestWorkshopHelpCtlWithoutCookie checks that help works through the
+// cookie-less workshopctl path. The test relies on the middleware-provided
+// workshop instance ID being present in the request context so the handler can
+// validate the caller and create an ephemeral hook context.
+func (s *apiSuite) TestWorkshopHelpCtlWithoutCookie(c *check.C) {
 	// Setup
 	s.daemon(c)
+	s.addWorkshopWithInstanceID("instance-id")
 	wctl := apiCmd("/v1/workshopctl")
 
 	buf := bytes.NewBufferString(`{"args":["-h"]}`)
 
 	req, err := s.createProjectsRequest("POST", "/v1/workshopctl", buf)
 	c.Assert(err, check.IsNil)
 
+	req = req.WithContext(context.WithValue(
+		req.Context(),
+		workshop.ContextWorkshopInstanceID,
+		"instance-id",
+	))
+
 	// Execute
 	rsp := v1PostWorkshopCtl(wctl, req, nil).(*resp)
 
@@ -41,3 +68,85 @@ func (s *apiSuite) TestWorkshopHelpCtlNoContext(c *check.C) {
 	_, err = rsp.MarshalJSON()
 	c.Assert(err, check.IsNil)
 }
+
+// TestWorkshopCtlRequiresInstanceIDWithoutCookie checks that workshopctl
+// requests without a hook cookie must identify their workshop instance.
+func (s *apiSuite) TestWorkshopCtlRequiresInstanceIDWithoutCookie(c *check.C) {
+	s.daemon(c)
+	wctl := apiCmd("/v1/workshopctl")
+	buf := bytes.NewBufferString(`{"args":["get-secret","sdk.secret"]}`)
+	req, err := s.createProjectsRequest("POST", "/v1/workshopctl", buf)
+	c.Assert(err, check.IsNil)
+
+	rsp := v1PostWorkshopCtl(wctl, req, nil).(*resp)
+
+	c.Check(rsp.Status, check.Equals, http.StatusBadRequest)
+}
+
+// TestWorkshopCtlRejectsUnknownInstanceID checks that a request without a hook
+// cookie cannot use an instance ID outside the requesting user's workshops.
+func (s *apiSuite) TestWorkshopCtlRejectsUnknownInstanceID(c *check.C) {
+	s.daemon(c)
+	wctl := apiCmd("/v1/workshopctl")
+	buf := bytes.NewBufferString(`{"args":["get-secret","sdk.secret"]}`)
+	req, err := s.createProjectsRequest("POST", "/v1/workshopctl", buf)
+	c.Assert(err, check.IsNil)
+	req = req.WithContext(context.WithValue(
+		req.Context(),
+		workshop.ContextWorkshopInstanceID,
+		"unknown-instance",
+	))
+
+	rsp := v1PostWorkshopCtl(wctl, req, nil).(*resp)
+
+	c.Check(rsp.Status, check.Equals, http.StatusForbidden)
+}
+
+// TestWorkshopCtlAcceptsOwnedInstanceID checks that a request without a hook
+// cookie may run after its instance ID is matched to the requesting user.
+func (s *apiSuite) TestWorkshopCtlAcceptsOwnedInstanceID(c *check.C) {
+	s.daemon(c)
+	s.addWorkshopWithInstanceID("instance-id")
+
+	wctl := apiCmd("/v1/workshopctl")
+	buf := bytes.NewBufferString(
+		`{"args":["get-secret","sdk.secret"],"stdin":"dGVzdA=="}`,
+	)
+	req, err := s.createProjectsRequest("POST", "/v1/workshopctl", buf)
+	c.Assert(err, check.IsNil)
+	req = req.WithContext(context.WithValue(
+		req.Context(),
+		workshop.ContextWorkshopInstanceID,
+		"instance-id",
+	))
+
+	rsp := v1PostWorkshopCtl(wctl, req, nil).(*resp)
+
+	c.Check(rsp.Status, check.Equals, http.StatusOK)
+}
+
+// TestWorkshopCtlRejectsUnknownCookie checks that an invalid cookie produces
+// a generic client error even when a valid workshop instance ID is supplied.
+func (s *apiSuite) TestWorkshopCtlRejectsUnknownCookie(c *check.C) {
+	s.daemon(c)
+	s.addWorkshopWithInstanceID("instance-id")
+	wctl := apiCmd("/v1/workshopctl")
+	buf := bytes.NewBufferString(
+		`{"context-id":"unknown-cookie","args":["get-secret","sdk.secret"]}`,
+	)
+	req, err := s.createProjectsRequest("POST", "/v1/workshopctl", buf)
+	c.Assert(err, check.IsNil)
+	req = req.WithContext(context.WithValue(
+		req.Context(),
+		workshop.ContextWorkshopInstanceID,
+		"instance-id",
+	))
+
+	rsp := v1PostWorkshopCtl(wctl, req, nil).(*resp)
+
+	c.Check(rsp.Status, check.Equals, http.StatusBadRequest)
+	c.Check(rsp.Type, check.Equals, ResponseTypeError)
+	c.Assert(rsp.Result, check.FitsTypeOf, &errorResult{})
+	c.Check(rsp.Result.(*errorResult).Message, check.Equals,
+		"invalid workshop cookie")
+}

internal/overlord/hookstate/ctlcmd/ctlcmd.go

@@ -20,6 +20,7 @@ package ctlcmd
 
 import (
 	"bytes"
+	"context"
 	"fmt"
 	"io"
 
@@ -97,7 +98,7 @@ type command interface {
 	setContext(context *hookstate.Context)
 	context() *hookstate.Context
 
-	Execute(args []string) error
+	Execute(ctx context.Context, args []string) error
 }
 
 type commandInfo struct {
@@ -142,7 +143,12 @@ func (f ForbiddenCommandError) Error() string {
 var nonRootAllowed = []string{"get-secret", "set-health"}
 
 // Run runs the requested command.
-func Run(context *hookstate.Context, args []string, uid uint32) (stdout, stderr []byte, err error) {
+func Run(
+	ctx context.Context,
+	hookContext *hookstate.Context,
+	args []string,
+	uid uint32,
+) (stdout, stderr []byte, err error) {
 	if len(args) == 0 {
 		return nil, nil, fmt.Errorf("workshopctl cannot run without args")
 	}
@@ -156,12 +162,14 @@ func Run(context *hookstate.Context, args []string, uid uint32) (stdout, stderr
 	// Create stdout/stderr buffers, and make sure commands use them.
 	var stdoutBuffer bytes.Buffer
 	var stderrBuffer bytes.Buffer
+	activeCommands := make(map[string]command, len(commands))
 	for name, cmdInfo := range commands {
 		cmd := cmdInfo.generator()
 		cmd.setName(name)
 		cmd.setStdout(&stdoutBuffer)
 		cmd.setStderr(&stderrBuffer)
-		cmd.setContext(context)
+		cmd.setContext(hookContext)
+		activeCommands[name] = cmd
 
 		theCmd, err := parser.AddCommand(name, cmdInfo.shortHelp, cmdInfo.longHelp, cmd)
 		theCmd.Hidden = cmdInfo.hidden
@@ -170,6 +178,14 @@ func Run(context *hookstate.Context, args []string, uid uint32) (stdout, stderr
 		}
 	}
 
+	parser.CommandHandler = func(_ flags.Commander, args []string) error {
+		cmd, ok := activeCommands[parser.Active.Name]
+		if !ok {
+			return fmt.Errorf("internal error: active command %q not found", parser.Active.Name)
+		}
+		return cmd.Execute(ctx, args)
+	}
+
 	_, err = parser.ParseArgs(args)
 	return stdoutBuffer.Bytes(), stderrBuffer.Bytes(), err
 }