feat: add environment aliases and consolidate portal parsing [REPORT-87]

[dougmartin]

Adds prod/staging/dev environment aliases so one word sets a portal and the report server it is paired with, and consolidates portal parsing behind a single type because adding aliases exposed that four different functions already turned user input into a portal.

What you get

cc-data login staging sets the portal and the staging report server, so a staging login cannot end up talking to production. The aliases work on --portal and --server, expanding independently (--portal staging --server dev is meaningful), and on the --portal flag of logout, reports list, reports jobs, and the MCP portal arguments.

The rule the whole change follows, and the one sentence worth remembering:

An environment alias names a network target. A portal that names something on disk is always a hostname.

So aliases are refused, not expanded, in a dataset ref and in default_portal. Expanding there would give one dataset folder two names; taking the alias literally would file data under a portal that can never hold a credential, which reads downstream as a NOT_AUTHENTICATED that logging in appears to fix and does not.

Why the diff is bigger than the feature

Four functions already converted user input into a portal, each with different acceptance rules, and one call site used none of them. Every bug found while building this was a call site reaching the wrong one. They are replaced by a config.Portal type that no other package can construct from a string, plus two parsers:

• ParsePortalTarget — a portal to talk to. Aliases expand; a near-miss alias like stagng is refused.
• ParsePortalIdentity — a portal that names something on disk. Aliases refused; single-label hosts preserved.

auth.ResolvePortalTarget adds the one exception both need: a host a stored credential vouches for is accepted, so a credential written by an older build can still be listed, used, revoked, and refreshed. dataset.Ref, the creds store, api.ForPortal and auth.LoginOptions all take config.Portal, so a portal cannot reach a credential key, a dataset folder, or an auth URL unparsed.

Security fix (predates this branch, present in 0.1.0)

cc-data dataset create ../wildfire resolved outside the configured data root, reaching MkdirAll and, via dataset delete, os.RemoveAll. It was reachable from a plain CLI argument and through the MCP dataset_create tool. url.Parse reports .. as the host of https://../x, and the folder encoding passed it straight through. The parsers now require a host to be shaped like one, which is also what guarantees a portal is a single path component.

Also fixed

• A named environment supplies its server for a portal no environment claims, so cc-data login dev --portal localhost:3005 stops exchanging its code at the production report server. The help text and researcher guide stated a precedence the code did not implement.
• default_portal is normalized once at load. Left raw, https://learn.concord.org and learn.concord.org named the same portal but two different folders, and the auto-named dataset create path filed data under the URL-shaped one, where dataset list could not see it.
• dataset show/delete/purge reach a dataset whose portal the parser refuses when the folder exists, so nothing dataset list shows is removable only with rm -rf.
• Portal hosts exclude underscores and IPv6 literals, the two shapes the folder encoding could not represent reversibly. A dataset under host_3000 was reported by dataset list as belonging to host:3000.
• auth status gains a SERVER column; a credential stored before the server was recorded shows -.
• An unreadable credential store no longer reports as a misspelled portal.

Tests

A portal matrix crossing every input shape (aliases short/long/cased/schemed, hostnames, URLs, loopback with and without ports, near-miss aliases, traversals, empty ports) against both parsers, one row each. Plus a property test that whatever either parser accepts stays a single path component inside the data root, an exact folder round-trip test, and a login precedence table covering every spelling of a portal against every server configuration.

Behavior changes (upgrade-visible)

Two things behave differently than 0.1.0 for an existing user; both are intended.

• login's report-server precedence. When --server is absent, the server is now chosen as: --server → the server paired with the resolved portal → the environment named as an argument → server_url → built-in default. On 0.1.0 an absent --server always meant server_url (else the default), so server_url applied to every login; now a known portal's pairing outranks it, and server_url only applies to a portal no environment claims. Concretely: with server_url: https://report-server.concordqa.org and a bare cc-data login (production portal), 0.1.0 logged into production against the QA server; this build logs into production against report-server.concord.org and prints a stderr line saying server_url was not used. Pass --server to override.

• Dataset refs and default_portal must be hostnames. An environment alias (prod/staging/dev), and a host shape 0.1.0 accepted but this build rejects (an underscore, an IPv6 literal), are refused when creating a dataset or as default_portal, with the hostname to use named in the error. Datasets already on disk under such a portal stay fully reachable (show/delete/purge/rename/edit/reindex/get/query); only create is strict. A default_portal the parser rejects now fails config.Load with the file path and remedy.

Reviewer notes

Addressed emcelroy's review in 322159e. The salvage exception (reach a dataset whose portal the strict parser refuses) now covers every command that names an existing dataset, not just show/delete/purge, and covers any folder-safe refusal, not just aliases; only create stays strict. So the earlier asymmetry note is resolved: the one rule is "create is strict; anything naming an existing dataset may reach a refused portal when the folder exists," enforced in one place (ParseRefForExisting → config.AdoptExistingPortal) and never for a traversal.

Not included, and deliberately so: the portal is still recorded only as a directory name rather than in the manifest. Restricting hosts to what the folder encoding can represent closes the correctness gap without a data migration.

[Copilot]

Pull request overview

Adds first-class environment aliases (prod/staging/dev) and consolidates all portal parsing behind a single config.Portal type, ensuring consistent normalization and safer handling across CLI, MCP tools, datasets, and credentials.

Changes:

• Introduces config.Portal plus ParsePortalTarget/ParsePortalIdentity and environment alias resolution for portal/server selection.
• Updates CLI + MCP entry points to use the unified portal parsing and to expand (or refuse) aliases according to the “target vs identity” rule.
• Hardens dataset/portal handling to prevent traversal and normalizes default_portal at load/save, with broad test coverage and docs updates.

Reviewed changes

Copilot reviewed 42 out of 42 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
README.md	Documents alias behavior and updated command forms.
internal/mcpserver/tools.go	MCP tool descriptions + portal resolution via auth.ResolvePortalTarget; ref parsing via config-aware parser.
internal/mcpserver/server.go	Adds “open existing dataset” path that can reach stranded on-disk portals.
internal/mcpserver/server_test.go	Updates tests to construct refs with config.Portal.
internal/fetch/report_test.go	Updates dataset fixtures to use config.Portal.
internal/duck/engine_test.go	Updates dataset fixtures/usages for config.Portal ref type.
internal/dataset/summary.go	Emits portal host string from config.Portal.
internal/dataset/summary_test.go	Updates dataset creation fixtures to use config.Portal.
internal/dataset/ref.go	Migrates dataset refs to config.Portal and adds config/existing ref parsers.
internal/dataset/ref_test.go	Adds traversal/alias refusal tests and “existing dataset” salvage coverage.
internal/dataset/merge_test.go	Updates dataset fixture portal to config.Portal.
internal/dataset/dataset.go	Updates AutoName to take config.Portal.
internal/dataset/dataset_test.go	Updates fixtures and AutoName calls for config.Portal.
internal/creds/creds.go	Moves credential store API to config.Portal inputs.
internal/creds/creds_test.go	Updates credential tests for config.Portal inputs.
internal/config/portal.go	Implements config.Portal, alias tables, target/identity parsing, and host syntax checks.
internal/config/portal_test.go	Adds portal matrix + property tests and environment alias tests.
internal/config/config.go	Normalizes and validates default_portal on load/save; adds DefaultPortalValue().
internal/config/config_test.go	Tests alias refusal + normalization of default_portal; extends server URL validation tests.
internal/claude/skill/SKILL.md	Updates skill guidance for env aliases and dataset-ref alias refusal.
internal/auth/status.go	Adopts stored portal strings into config.Portal for checking legacy entries.
internal/auth/portal.go	Adds ResolvePortalTarget to unify portal resolution with legacy-credential exception.
internal/auth/portal_test.go	Tests alias expansion and legacy-credential exception behavior.
internal/auth/pkce_test.go	Updates auth URL building to pass portal origin (scheme-bearing) value.
internal/auth/logout.go	Changes Logout signature to take config.Portal.
internal/auth/login.go	Changes LoginOptions portal representation; passes portal origin to auth URL builder.
internal/auth/integration_test.go	Updates integration tests for config.Portal and updated store APIs.
internal/api/portal.go	Changes api.ForPortal to take config.Portal.
internal/api/portal_test.go	Updates tests for new api.ForPortal signature.
docs/researcher-guide.md	Documents env alias usage and precedence rules; clarifies dataset-ref hostname requirement.
cmd/uninstall.go	Adopts stored portal strings for revocation during uninstall.
cmd/reports.go	Allows `--portal <portal
cmd/reports_test.go	Adds tests for reports portal resolution with default fallback.
cmd/repl_engine_test.go	Updates dataset fixture portal to config.Portal.
cmd/logout.go	Allows `--portal <portal
cmd/login.go	Adds positional environment arg, independent alias expansion for --portal/--server, and precedence logic.
cmd/login_test.go	Adds comprehensive precedence tests and env-arg validation tests.
cmd/dataset.go	Uses parsed default portal for autonaming; routes delete/purge through “existing dataset” ref parsing.
cmd/dataset_show.go	Routes show through “existing dataset” ref parsing.
cmd/common.go	Centralizes dataset ref parsing through config-aware parsers; adds existing-ref resolver.
cmd/auth.go	Adds SERVER column to auth status output and renders missing server as “-”.
cmd/auth_test.go	Tests SERVER column rendering behavior.

Comments suppressed due to low confidence (2)

internal/dataset/ref.go:73

• If splitRef fails while recovering from an alias refusal, returning the earlier alias error hides the actual parsing failure (e.g., missing name). Return serr so the user gets the correct error.
  internal/dataset/ref.go:77
• If buildRef fails (e.g., invalid/empty dataset name), returning the earlier alias error is misleading. Return cerr so the caller sees the actual validation failure.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[emcelroy]

Looks good to me 👍 The report below was generated with Claude. I don't think any of the issues raised are critical (and #1 is possibly not an actual problem at this point), but you may want to consider addressing them.

The design is sound. config.Portal with an unexported field is what makes this more than a rename: no other package can mint one from a string, so the "four parsers with four acceptance rules" problem cannot recur by accident, and AdoptStoredPortal is the single named escape hatch, correctly used at all three of its call sites. Splitting target from identity is the right axis — it decides the hard cases instead of restating them. The traversal fix is real: on main, NormalizePortal("../wildfire") returns ".." with no error and the folder encoding passes it into Ref.Dir, reaching MkdirAll and os.RemoveAll; both parsers now refuse it. Restricting hosts to what Folder can encode reversibly, rather than escaping around the problem, is the right call given the portal is recorded only as a directory name.

The findings below are one correctness gap that contradicts a claim in the PR description, one consistency gap, and a set of smaller items.

---

1. (Medium) Legacy portal folders with underscores or IPv6 literals are listed but unreachable

The PR states that "nothing dataset list shows is removable only with rm -rf." That holds for the alias refusal, which is the only error ParseRefForExisting salvages. It does not hold for the syntax refusals this PR introduces. Underscored hosts and bracketed IPv6 literals were accepted by main's NormalizePortal, so 0.1.0 users can have such folders on disk today:

dataset list shows:  a_b.concord.org/wildfire
ParseRefForExisting("a_b.concord.org/wildfire")
  -> invalid portal "a_b.concord.org": "a_b.concord.org" is not a hostname

show, delete, and purge all refuse it. rm -rf is the only way out — the exact scenario the exception exists to prevent.

Note the near-miss that makes this easy to overlook: a folder named host_3000 does work, because decodePortalFolder renders it as host:3000, which re-parses and re-encodes back to host_3000. Only underscores outside a trailing _<digits>, and [__1]-style IPv6 folders, strand.

Fix: widen the salvage to any portal refusal where the raw portal text, taken as a literal folder name, contains no path separator, is not . or .., and exists under the data root. The traversal guard is preserved by construction. Add a test asserting a pre-existing a_b.concord.org folder is reachable.

2. (Medium–Low) The "already exists" exception covers 3 operations out of 14

The rule is "an operation may name a refused portal exactly when the thing it names already exists." It is applied to show, delete, and purge. These also name a thing that already exists, and still refuse:

Path	Call site
dataset rename	cmd/dataset.go:119
dataset edit	cmd/dataset.go:85
dataset reindex	cmd/dataset_reindex.go:19
query / repl	cmd/query.go:88
get report/answers/history/attachments	cmd/get.go:28
MCP dataset_rename, dataset_edit, dataset_reindex	tools.go:142, :156, :198
MCP query	tools.go:264
MCP get_report, get_answers, get_history, get_attachments	via openForFetch → openDataset (server.go:110)

A dataset at staging/wildfire (creatable in 0.1.0) can be shown and deleted but not queried, renamed, or reindexed. rename is the sharpest omission: moving the dataset out from under the bad portal name is the natural remedy, and it refuses.

Fix the CLI and MCP sides together — fixing only MCP would leave the CLI inconsistent with its own server. If you keep the surface narrow instead, name in ParseRefForExisting's doc comment which operations are deliberately excluded; as written the omission reads as an oversight.

Related: rename can't change a dataset's portal anyway, so dataset move <ref> <portal>/<name> may be the real missing escape hatch. Out of scope; worth a ticket.

3. (Low) A credential stored under a literal alias name is unreachable

auth.ResolvePortalTarget makes its credential-vouching exception for *UnshapedHostError only. *AliasPortalError never arises on the target path, because ParsePortalTarget expands the alias before any shape check. A 0.1.0 credential stored under the literal host staging (login --portal staging was accepted on main) is therefore:

• visible in auth status,
• not revocable via logout --portal staging — that resolves to learn.portal.staging.concord.org and reports no credential,
• not usable by reports list --portal staging, same reason.

uninstall can still revoke it (it uses AdoptStoredPortal), which bounds the damage. But this is the failure mode ResolvePortalTarget's doc comment says the exception prevents; the exception just doesn't cover the alias-shaped case. At minimum, scope the comment to match. To fix: check the credential store before alias expansion and prefer a stored literal match, with a one-line note saying which was used.

4. (Low) server_url is now outranked for known portals — call this out in release notes

Pre-branch, an absent --server meant cfg.ServerOrigin(). Now the portal pairing outranks it (cmd/login.go:175). A user with server_url: https://report-server.concordqa.org and no --portal used to log into learn.concord.org against the QA server; the same command now targets report-server.concord.org.

The old --server help text advertised the QA origin, so "set server_url to the QA server" was a plausible thing to have done. The design is right and the stderr notice is well placed — it prints before the browser opens or a token is pasted, so it is actionable. But a stderr line on a command run fifty times is easy to miss; this belongs in the changelog.

5. (Low) The "server_url was not used" notice fires spuriously on a case-differing origin

cmd/login.go:177 compares cfg.ServerURL != rawServer as strings. cfg.ServerURL has already been through ValidateServerURL at load, so a trailing slash or /api path is gone. Case is not, because ValidateServerURL returns u.Scheme + "://" + u.Host and discards the lowercased host it computed for the allowlist check:

ValidateServerURL("https://report-server.concord.org/")    -> "https://report-server.concord.org"    (== default)
ValidateServerURL("https://REPORT-SERVER.CONCORD.ORG")     -> "https://REPORT-SERVER.CONCORD.ORG"    (!= default)
ValidateServerURL("https://Report-Server.Concord.org/api") -> "https://Report-Server.Concord.org"    (!= default)

So server_url: https://REPORT-SERVER.CONCORD.ORG prints "the configured server_url … was not used" against the server it in fact names, on every login to a portal in the alias table.

Fix in ValidateServerURL, not at the comparison site: lowercase the host in the returned origin. That also corrects the origin written to the credential store — which now renders in the new SERVER column and is the base URL api.ForPortal uses for every subsequent request on that portal. Re-validating cfg.ServerURL at the comparison site does not fix this, since ValidateServerURL is idempotent on a mixed-case host. Add a mixed-case case to TestValidateServerURL.

This behavior predates the branch; the PR makes it user-visible by building a comparison and a notice on top of it.

6. (Low) A bad default_portal blocks every command that loads config

Load errors on an unparseable default_portal, so auth status, dataset list, and every other command calling config.Load/loadRuntime fail for a problem most of them don't care about. PersistentPreRunE doesn't load config, so cc-data version and --help still work — but neither tells you what's wrong, and every command that could is one that just failed. There is no cc-data config unset default_portal, so the remedy is hand-editing JSON.

Probability is low (nothing writes default_portal). The error naming the file is most of the mitigation. Either demote to a stderr warning treating the default as unset, or append the remedy to the message ("remove or fix default_portal in <path>").

7. (Low, tests) TestParseRefStaysInsideTheDataRoot's assertion is unreachable

ref_test.go:84 passes config.Portal{} as the default portal and continues on refusal, so all four inputs bail before filepath.HasPrefix runs:

refused: "../wildfire"        -> invalid portal "..": ".." is not a hostname
refused: "../../wildfire"     -> invalid portal "..": ".." is not a hostname
refused: "./wildfire"         -> invalid portal ".": "." is not a hostname
refused: "..%2f..%2fwildfire" -> no portal in ref and no default_portal configured
iterations reaching the containment assertion: 0/4

It's a valid conditional invariant — a parser regression would make the assertion fire — but today it adds nothing over TestParseRef's wantErr rows for the same inputs, never calls Ref.Dir, and its last row fails on the missing default portal rather than on name validation. The ..%2fwildfire row in TestParseRef (ref_test.go:37) has the same problem.

The property itself is genuinely covered in the config package by TestPortalFolderStaysOneComponent, which drives every portal the matrix's parsers accept. What's untested is Ref.Dir's own join for an accepted ref. Give the test a non-zero default portal and at least one accepted case.

8. Nits

• Dead condition. portal.go:238: strings.Contains(h, ":") can never be true — h is post-splitHostPort, and checkHostSyntax rejected bracketed IPv6 upstream. Drop it or say why it stays.
• Zero Portal is externally constructible. config.Portal{} compiles anywhere, and Portal{}.Folder() is "", collapsing Ref.Dir to <root>/datasets/<name> — a location dataset list never scans. Not currently reachable (cmd/dataset.go:58 is IsZero-guarded). Guard in buildRef, which already returns an error and is the only path a Ref takes; Ref.Dir returns a bare string, so a guard there would need a panic or a signature change.
• hostSyntax is looser than a hostname. a..b and a-.b are accepted. Harmless — both are single reversible path components, which is the property that matters — but the comment calls it "a hostname or dotted IPv4 literal." Tighten the regexp to per-label matching, or soften the comment.
• parseHost's doc comment says "port preserved for dev portals" (portal.go:156); the port is preserved for any host — the matrix has 192.168.1.10:3000 and portal.example.org:8443. Drop "for dev portals".
• auth status test isn't pinned to its column. auth_test.go:42 asserts strings.Contains(lines[2], " - "). It works today (no neighbouring column renders a bare -, and the fixture uses time.Unix(0, 0) rather than a zero time), but it's one new --rendering column away from vacuous. strings.Fields(lines[2])[1] != "-" costs nothing.
• DefaultPortalValue() re-parses on every call though Load already normalized. Caching the parsed value on Config would make the invariant structural rather than commented.
• Pre-existing, drive-by: SKILL.md line 16 says "never read manifest.json directly"; line 130 says "You may auto-read the dataset show --json summary and manifest." Both lines are on main and neither is in this diff, but the fix is four words.

[dougmartin]

Thanks @emcelroy, thorough report. All addressed in 322159e; details below.

#1 (Medium) legacy underscore/IPv6 folders unreachable — right, and this was the sharp one: the "nothing dataset list shows needs rm -rf" claim held for the alias refusal but not for the syntax refusals this branch introduced. ParseRefForExisting now salvages any portal refusal via a new config.AdoptExistingPortal, which accepts a value the strict parser rejects (alias, underscore, IPv6 literal) only when it is a single safe path component and a folder exists under it. Traversal refusals still stand. Test asserts a pre-existing a_b.concord.org folder is reachable.

#2 (Medium-Low) 3-of-14 coverage — extended rather than documented. Every command that names an existing dataset now routes through the salvaging parser: rename, edit, reindex, get, query, plus the MCP equivalents; only create stays strict. The asymmetry is gone. (Agreed a dataset move <ref> <portal>/<name> is the real missing escape hatch for changing a portal, filing that separately.)

#3 (Low) literal-alias credential — scoped the comment rather than changing behavior, because ResolvePortalTarget also feeds login, which must expand staging to the canonical host; preferring the literal there would perpetuate the legacy credential on every fresh login. The doc now names the gap and its remedy (uninstall, which already adopts stored hosts directly).

#4 (Low) server_url precedence — documented as a "Behavior changes" section in the PR description (no changelog in-repo). Good call, the stderr notice alone is too easy to miss.

#5 (Low) case-differing origin notice — fixed in ValidateServerURL as you suggested, lowercasing the host in the returned origin, which also corrects the stored SERVER value. Added mixed-case rows to TestValidateServerURL.

#6 (Low) bad default_portal — kept fail-fast but appended the remedy: ... (remove or fix default_portal to continue), alongside the file path.

#7 (tests) unreachable assertion — TestParseRefStaysInsideTheDataRoot now runs its containment check on accepted refs (with a default portal set) and calls Ref.Dir; the ..%2f row now exercises name validation.

#8 nits — dead colon check dropped; buildRef rejects a zero portal (single Ref choke point); hostSyntax and parseHost comments corrected; auth-status test pinned to the SERVER column via strings.Fields; SKILL.md manifest contradiction reconciled. Left DefaultPortalValue's re-parse as-is (caching risks staleness since Save mutates the field) and the pre-existing SKILL fix folded into #8's reconciliation.