fix(sysmlapi): keep the bearer token on the configured server and the branch read honest

changes/unreleased/sysmlapi-client-hardening.fixed.md

@@ -0,0 +1,4 @@
+- **A SysML v2 API server cannot have the bearer token paged on to another host.** A `Link: rel="next"` header naming a page on another server, on another port, or over plaintext is refused with the error a redirect there gets ("the token is for *host* only"); the page is never requested with the token. Without a token the link is followed as before.
+- **The repository URL the environment supplies is held to the plaintext rule.** `FLEXO_SYSMLV2_URL` naming an `http://` server off this machine is refused by `%repo`, `%projects`, `%load` and `%publish` before any request goes out — the same refusal `%repo <url>` gives, lifted the same way by `FLEXO_ALLOW_PLAIN_HTTP=1` — instead of sending `FLEXO_INTEROP_TOKEN` in the clear.
+- **A branch read before its first commit is still checked for a moved head.** A change set computed against a branch with no head commit is refused as a stale branch when another writer made the first commit in between, instead of being posted onto the commit it never saw.
+- **`%publish` by name reaches the project the session loaded by id.** When the session tracks a project of that name on the server, the publish resolves it by the stored id, so a second project with the same name no longer makes the name ambiguous; `--project` naming another project and the refusal of a shared name no session project matches are unchanged.

docs/guide/12-jupyter.md

@@ -264,7 +264,7 @@ environment of the notebook server:
 | `FLEXO_SYSMLV2_URL` | `http://localhost:9000` | the SysML v2 API endpoint, by default `http://localhost:8083` |
 | `FLEXO_INTEROP_TOKEN` | unset | the bearer token; never put it in a cell |
 | `FLEXO_SYSMLV2_ORG` | unset | the organization, by default `sysmlv2` |
-| `FLEXO_ALLOW_PLAIN_HTTP` | unset on this machine | `1` to allow a plaintext `http://` server on another |
+| `FLEXO_ALLOW_PLAIN_HTTP` | unset on this machine | `1` to allow a plaintext `http://` server on another, whether `FLEXO_SYSMLV2_URL` or `%repo` names it; the token still never follows a redirect or a next-page link to another server |
 
 Then, as the pilot's notebooks do:
 

docs/reference/repl-commands.md

@@ -124,7 +124,7 @@ into the parts it holds (`car.fl.hub`, `#3.fl`, `car.wheels[2]`).
 
 | Command | Description |
 |---------|-------------|
-| `%repo [<base path>]` | Show or set the base URL of the SysML v2 API server the repository commands address, seeded from `FLEXO_SYSMLV2_URL` (default `http://localhost:8083`). A bearer token, when the server wants one, is read from `FLEXO_INTEROP_TOKEN`; it is never printed, and nothing is persisted. A plaintext `http://` URL off this machine is refused unless `FLEXO_ALLOW_PLAIN_HTTP=1` |
+| `%repo [<base path>]` | Show or set the base URL of the SysML v2 API server the repository commands address, seeded from `FLEXO_SYSMLV2_URL` (default `http://localhost:8083`). A bearer token, when the server wants one, is read from `FLEXO_INTEROP_TOKEN`; it is never printed, and nothing is persisted. A plaintext `http://` URL off this machine is refused unless `FLEXO_ALLOW_PLAIN_HTTP=1`, whether `%repo` names it or `FLEXO_SYSMLV2_URL` does; and the token stays on the server it was given for: a redirect or a next-page link to another host, another port or plaintext is refused, not followed with the token |
 | `%projects` | List the repository's projects, `<name> (<id>)` one per line, every page of them; `no projects` for an empty repository |
 | `%publish [-d] [--project=<project name>] [--branch=<branch name>] <name>` | Publish the elements rooted in `<name>` — a qualified name resolved in the session — as SysML v2 API JSON: when no project is named after it (or after `--project`) a project is created and its first commit made, otherwise a commit on the branch `--branch` names, or the default branch, holding what differs from the branch head under that root — elements under other roots are left in place, and elements are deleted only from a branch the session loaded or published: the commit id and the counts of elements created, updated and deleted are reported, with what was left in place, `nothing to publish` when none differ. An option given twice is a usage error. `-d` sends every derived property the exporter computes; without it the ones the reader recomputes are left out. A project with the same name twice is refused naming both ids |
 | `Tab` | Complete meta commands, symbol names (after `%print`, `%instantiate`, `%features` …; a name that needs quoting is offered in quotes, `Q::'the ra` completing to `Q::'the rack'`), object references where a command takes one (`#` offers the ids there are; `car.` offers the object-holding features of `car` — the same ones a path may pass through — a multi-valued one as `car.wheels[1]`, `car.wheels[2]` …; completing reads and materializes nothing, so a part no command has reached yet is offered by type, and only the elements reading it would hold: those the features subsetting it contribute, then anonymous ones up to its lower bound — so an optional part (`spare : Wheel[0..1]`) or an abstract one, which hold only what subsets them, is offered only once something does), the form after `%render <name>` and the palette after `%render <name> dot`, file paths after `%load` and `%save`, and the flags of `%load` and `%publish` with the repository's project names after `--name=`, `--id=` and `--project=` and after `%load` |

internal/frontend/repl/replext/repository/repository.go

@@ -92,6 +92,23 @@ func findProject(ctx context.Context, c *sysmlapi.Client, id, name string) (sysm
 	return sysmlapi.Project{}, &modelsync.AmbiguousNameError{Name: name, IDs: ids}
 }
 
+// publishTarget is the project a publish by name means: the one the session
+// tracks under that name while the server still has it by that name, else
+// whatever the name resolves to now — a renamed or deleted one is no match.
+func publishTarget(ctx context.Context, c *sysmlapi.Client, id, name string) (sysmlapi.Project, error) {
+	if id != "" {
+		project, err := findProject(ctx, c, id, "")
+		var missing *NotFoundError
+		switch {
+		case err == nil && project.Name == name:
+			return project, nil
+		case err != nil && !errors.As(err, &missing):
+			return sysmlapi.Project{}, err
+		}
+	}
+	return findProject(ctx, c, "", name)
+}
+
 // findBranch resolves a branch by name or id, or the project's default branch
 // when none is named.
 func findBranch(ctx context.Context, c *sysmlapi.Client, project sysmlapi.Project, nameOrID string) (sysmlapi.Branch, error) {
@@ -195,7 +212,13 @@ func (repository) Publish(ctx context.Context, base string, req replext.PublishR
 	result := &replext.PublishResult{}
 	// State from another server is no history of this one, whatever ids match.
 	known := req.State != nil && req.State.Base == base
-	project, err := findProject(ctx, c, "", name)
+	// The project the session tracks under this name is the one meant, by id,
+	// so a namesake elsewhere on the server does not make the name ambiguous.
+	id := ""
+	if known && req.State.ProjectName == name {
+		id = req.State.ProjectID
+	}
+	project, err := publishTarget(ctx, c, id, name)
 	var missing *NotFoundError
 	switch {
 	case errors.As(err, &missing):

internal/frontend/repl/repository.go

@@ -30,12 +30,17 @@ const (
 const completionTimeout = 2 * time.Second
 
 // repoBase is the base URL the repository commands address: the one %repo set,
-// else the environment's.
-func (s *Session) repoBase() string {
+// else the environment's, held to the same transport rule %repo applies to a
+// URL it is given, so a token never leaves on a URL that was never checked.
+func (s *Session) repoBase() (string, error) {
 	if s.repoURL != "" {
-		return s.repoURL
+		return s.repoURL, nil
 	}
-	return replext.Repo().DefaultURL()
+	base := replext.Repo().DefaultURL()
+	if err := replext.Repo().CheckURL(base); err != nil {
+		return "", err
+	}
+	return base, nil
 }
 
 func (s *Session) metaRepositoryCommand(fields []string, _ string) (metaResult, bool) {
@@ -65,7 +70,11 @@ func usageError(lines ...string) metaResult {
 func (s *Session) doRepo(args []string) metaResult {
 	switch len(args) {
 	case 0:
-		return metaOut([]string{"API base path: " + s.repoBase()}, false, nil)
+		base, err := s.repoBase()
+		if err != nil {
+			return metaOut(nil, false, err)
+		}
+		return metaOut([]string{"API base path: " + base}, false, nil)
 	case 1:
 		base := strings.TrimRight(nameText(args[0]), "/")
 		if err := replext.Repo().CheckURL(base); err != nil {
@@ -81,7 +90,11 @@ func (s *Session) doProjects(args []string) metaResult {
 	if len(args) != 0 {
 		return usageError(usageProjects)
 	}
-	projects, err := replext.Repo().Projects(s.command.context(), s.repoBase())
+	base, err := s.repoBase()
+	if err != nil {
+		return metaOut(nil, false, err)
+	}
+	projects, err := replext.Repo().Projects(s.command.context(), base)
 	if err != nil {
 		return metaOut(nil, false, err)
 	}
@@ -178,7 +191,11 @@ func (s *Session) doLoadRepository(args []string) metaResult {
 	if req.Name != "" && req.ProjectID != "" {
 		return usageError(usageLoadRepo, "name the project once: by --id, by --name or by itself")
 	}
-	loaded, err := replext.Repo().Load(s.command.context(), s.repoBase(), req)
+	base, err := s.repoBase()
+	if err != nil {
+		return metaOut(nil, false, err)
+	}
+	loaded, err := replext.Repo().Load(s.command.context(), base, req)
 	if err != nil {
 		return metaOut(nil, false, err)
 	}
@@ -243,7 +260,11 @@ func (s *Session) doPublish(args []string) metaResult {
 	req.Root = fqn
 	req.Source = []byte(s.text())
 	req.State = s.repoState
-	published, err := replext.Repo().Publish(s.command.context(), s.repoBase(), req)
+	base, err := s.repoBase()
+	if err != nil {
+		return metaOut(nil, false, err)
+	}
+	published, err := replext.Repo().Publish(s.command.context(), base, req)
 	if err != nil {
 		return metaOut(nil, false, err)
 	}
@@ -305,9 +326,13 @@ func (s *Session) projectIDs() []string {
 }
 
 func (s *Session) projectField(field func(replext.ProjectInfo) string) []string {
+	base, err := s.repoBase()
+	if err != nil {
+		return nil
+	}
 	ctx, cancel := context.WithTimeout(context.Background(), completionTimeout)
 	defer cancel()
-	projects, err := replext.Repo().Projects(ctx, s.repoBase())
+	projects, err := replext.Repo().Projects(ctx, base)
 	if err != nil {
 		return nil
 	}

internal/frontend/repl/repository_test.go

@@ -503,3 +503,108 @@ func TestRepositoryOptionsAreGivenOnce(t *testing.T) {
 		}
 	}
 }
+
+// The environment's URL is held to the rule %repo holds a URL it is given to:
+// a plaintext server off this machine gets no request, so no token in the clear.
+func TestDefaultURLIsHeldToThePlaintextRule(t *testing.T) {
+	if replext.Repo() == nil {
+		t.Fatal("no repository extension is linked")
+	}
+	t.Setenv("FLEXO_SYSMLV2_URL", "http://models.example.com/api")
+	t.Setenv("FLEXO_INTEROP_TOKEN", "secret")
+	t.Setenv("FLEXO_ALLOW_PLAIN_HTTP", "")
+	s := NewSession()
+	s.Submit(vehicles)
+	for _, line := range []string{"%repo", "%projects", "%load --id=project-0001", "%publish Vehicles"} {
+		err := metaErr(t, s, line)
+		if !strings.Contains(err.Error(), "FLEXO_ALLOW_PLAIN_HTTP") || !strings.Contains(err.Error(), "http://models.example.com/api") {
+			t.Errorf("%s: the environment's plaintext URL was taken: %v", line, err)
+		}
+		if strings.Contains(err.Error(), "secret") {
+			t.Errorf("%s: the refusal names the token: %v", line, err)
+		}
+		if strings.Contains(err.Error(), "did not answer") {
+			t.Errorf("%s: a request went out: %v", line, err)
+		}
+	}
+	if got := s.Complete("%load Veh", len("%load Veh")).Candidates; len(got) != 0 {
+		t.Errorf("completion asked the refused server: %q", got)
+	}
+
+	if got := joined(runMeta(t, s, "%repo http://127.0.0.1:1/api")); got != "API base path: http://127.0.0.1:1/api" {
+		t.Errorf("%%repo <loopback url> = %q", got)
+	}
+	if err := metaErr(t, s, "%projects"); !strings.Contains(err.Error(), "did not answer") {
+		t.Errorf("a loopback URL set by %%repo was still refused: %v", err)
+	}
+
+	t.Setenv("FLEXO_ALLOW_PLAIN_HTTP", "1")
+	if got := joined(runMeta(t, NewSession(), "%repo")); got != "API base path: http://models.example.com/api" {
+		t.Errorf("with the opt-in, %%repo = %q", got)
+	}
+}
+
+// A project the session loaded by id is the one a publish by its name means,
+// even when another project on the server has the same name.
+func TestPublishByNameUpdatesTheLoadedNamesake(t *testing.T) {
+	s, api := repoSession(t)
+	s.Submit(vehicles)
+	runMeta(t, s, "%publish Vehicles")
+	namesake := api.addProject("Vehicles")
+	held := api.elementCount("project-0001", "branch-0002")
+
+	untracked := NewSession()
+	untracked.Submit(vehicles)
+	if err := metaErr(t, untracked, "%publish Vehicles"); !strings.Contains(err.Error(), "project-0001") || !strings.Contains(err.Error(), namesake.id) {
+		t.Errorf("without a loaded project, the shared name is not refused naming both: %v", err)
+	}
+
+	fresh := NewSession()
+	runMeta(t, fresh, "%load --id=project-0001")
+	fresh.Submit("package Vehicles { part def Wheel { attribute radius : ScalarValues::Real; } part def Car { part wheels : Wheel[4]; } part def Bus; }")
+	out := runMeta(t, fresh, "%publish Vehicles")
+	if len(out) != 1 || !strings.Contains(out[0], "of Vehicles (project-0001)") {
+		t.Fatalf("publish by name after loading by id:\n%s", joined(out))
+	}
+	if now := api.elementCount("project-0001", "branch-0002"); now <= held {
+		t.Errorf("the loaded project holds %d elements, had %d; Bus was not added", now, held)
+	}
+	if api.elementCount(namesake.id, namesake.defaults) != 0 || len(api.order) != 2 {
+		t.Errorf("the namesake received elements or a project was created: %d projects", len(api.order))
+	}
+	fresh.Submit("package Spare { part def Boat; }")
+	out = runMeta(t, fresh, "%publish --project=Vehicles Spare")
+	if len(out) < 1 || !strings.Contains(out[0], "of Vehicles (project-0001)") {
+		t.Errorf("--project naming the tracked project's name:\n%s", joined(out))
+	}
+	if len(api.order) != 2 {
+		t.Errorf("--project=Vehicles created a project: %d projects", len(api.order))
+	}
+
+	// Renamed on the server, the tracked project no longer answers to the name:
+	// the namesake, now alone under it, is the one published to.
+	api.mu.Lock()
+	api.projects["project-0001"].name = "Fleet"
+	api.mu.Unlock()
+	out = runMeta(t, fresh, "%publish Vehicles")
+	if len(out) != 1 || !strings.Contains(out[0], "of Vehicles ("+namesake.id+")") {
+		t.Fatalf("publish by a name the tracked project lost:\n%s", joined(out))
+	}
+	if api.elementCount(namesake.id, namesake.defaults) == 0 || len(api.order) != 2 {
+		t.Errorf("the namesake holds nothing or a project was created: %d projects", len(api.order))
+	}
+
+	// Deleted on the server, likewise: no project is created over the namesake.
+	api.mu.Lock()
+	delete(api.projects, "project-0001")
+	api.order = api.order[1:]
+	api.mu.Unlock()
+	fresh.Submit("package Vehicles { part def Wheel { attribute radius : ScalarValues::Real; } part def Car { part wheels : Wheel[4]; } part def Bus; part def Van; }")
+	out = runMeta(t, fresh, "%publish Vehicles")
+	if len(out) != 1 || !strings.Contains(out[0], "of Vehicles ("+namesake.id+")") {
+		t.Fatalf("publish by name after the tracked project was deleted:\n%s", joined(out))
+	}
+	if len(api.order) != 1 {
+		t.Errorf("a project was created over the namesake: %d projects", len(api.order))
+	}
+}

internal/translate/interop/sysmlapi/client.go

@@ -124,18 +124,28 @@ func (c *Client) checkRedirect(req *http.Request, via []*http.Request) error {
 	if len(via) >= maxRedirects {
 		return fmt.Errorf("stopped after %d redirects", maxRedirects)
 	}
+	from := via[len(via)-1].URL
+	if err := c.tokenStays(from, req.URL); err != nil {
+		return fmt.Errorf("redirect from %s refused: %w", from, err)
+	}
+	return nil
+}
+
+// tokenStays is the rule a request's successor — a redirect's target, a linked
+// next page — is held to when there is a bearer token to carry: no plaintext,
+// no leaving https once there, and no other server than the configured one.
+func (c *Client) tokenStays(from, to *url.URL) error {
 	if c.cfg.Token == "" {
 		return nil
 	}
-	from := via[len(via)-1].URL
-	if err := CheckURL(req.URL.String()); err != nil {
-		return fmt.Errorf("redirect from %s refused: %w", from, err)
+	if err := CheckURL(to.String()); err != nil {
+		return err
 	}
-	if from.Scheme == "https" && req.URL.Scheme != "https" {
-		return fmt.Errorf("redirect from %s to %s refused: the token stays on https", from, req.URL.Scheme)
+	if from.Scheme == "https" && to.Scheme != "https" {
+		return fmt.Errorf("%s: the token stays on https", to.Scheme)
 	}
-	if base, err := url.Parse(c.cfg.BaseURL); err == nil && !sameServer(req.URL, base) {
-		return fmt.Errorf("redirect from %s to %s refused: the token is for %s only", from, req.URL.Host, base.Host)
+	if base, err := url.Parse(c.cfg.BaseURL); err == nil && !sameServer(to, base) {
+		return fmt.Errorf("%s: the token is for %s only", to.Host, base.Host)
 	}
 	return nil
 }
@@ -377,7 +387,11 @@ func (c *Client) paged(ctx context.Context, target string, what string, each fun
 		}
 		// A server linking its pages is followed to the end whatever each
 		// page holds; without a link, a short page is the last.
-		if linked := nextLink(header, next); linked != "" && linked != next {
+		linked, err := c.nextPage(header, next)
+		if err != nil {
+			return err
+		}
+		if linked != "" && linked != next {
 			next = linked
 			continue
 		}
@@ -404,6 +418,28 @@ func withPaging(target, after string) string {
 	return out
 }
 
+// nextPage is the page a response links as next, held to the rule a redirect
+// is: a link to another server, or into the clear, is refused rather than
+// followed with the token.
+func (c *Client) nextPage(header http.Header, requested string) (string, error) {
+	linked := nextLink(header, requested)
+	if linked == "" {
+		return "", nil
+	}
+	from, err := url.Parse(requested)
+	if err != nil {
+		return "", err
+	}
+	to, err := url.Parse(linked)
+	if err != nil {
+		return "", err
+	}
+	if err := c.tokenStays(from, to); err != nil {
+		return "", fmt.Errorf("next page linked from %s refused: %w", from, err)
+	}
+	return linked, nil
+}
+
 // nextLink is the rel="next" target of a Link header, resolved against the
 // request's URL; empty when the response links no next page.
 func nextLink(header http.Header, requested string) string {
@@ -569,7 +605,10 @@ func (c *Client) Elements(ctx context.Context, project, commit string, size int)
 		if len(page) == 0 || listing.Responses > maxPages {
 			return listing, nil
 		}
-		linked := nextLink(header, target)
+		linked, err := c.nextPage(header, target)
+		if err != nil {
+			return listing, err
+		}
 		switch {
 		case linked != "" && linked != target:
 			// A server linking its pages names the continuation itself,