From ad05d8056bb53e878db0a54b1fd9c6b3ac7352db Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 01:05:31 +0000 Subject: [PATCH 1/2] Make file shares a first-class source type in the console (#196) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The API has supported `fileshare` since #145, but the console's form had fields for Confluence and Jira only, so a share could be configured through the API and nowhere else. #187 stopped that from 500ing; a graceful refusal is not support. The share's own fields are now on the form — protocol, mount path, roots, include/exclude globs, symlink policy and the per-file ceiling — and `fileshare` joins the type select, which a test now holds against the API's own `SUPPORTED_SOURCE_TYPES` rather than a list repeated in the test. It is deliberately not the Confluence form with different labels. A share has no base URL, no Cloud-vs-DC choice, and no credential: the mount carries the host, the share name and the authentication, so those fields are hidden for this type and the form says why rather than leaving a gap where the token box is on every other type. `_connection_form` returns early for it, because `FileshareConnection` forbids extras and would reject a blob carrying `base_url` or `include_comments` — which every save posts, since all three scope blocks stay in the DOM. Three consequential details: * `base_url` and `credential` become optional form parameters. The browser posts neither for a share, and a missing required `Form()` is FastAPI's own 422 — an error page rather than the form the analyst is looking at. * `max_file_bytes` is read as text and converted here for the same reason: a mistyped ceiling re-renders the form with a message. * The connectivity-test button is not offered where no probe exists. `probe_source` already refused a share and said why; the console showed a button that could only ever produce that refusal. `PROBEABLE_TYPES` is the API's own answer to "is there a check for this type", asked before offering. The chip helpers do not upper-case: roots and globs are paths, where case is significant, unlike a space or project key. A leading slash is trimmed, since a root is relative to the mount and the API answers 422 for one. Two tests that pinned #187's refusal are superseded — one by the new "editing a share saves it", the other rewritten to cover the failure that is still reachable: a hand-posted body that does not match its type. Refs #145. Claude-Session: https://claude.ai/code/session_012sohE85sRDt6t2w3936rGJ Co-authored-by: Claude --- CHANGELOG.md | 9 + apps/api/src/iceberg_api/sources/probe.py | 5 + .../api/src/iceberg_api/web/routes/sources.py | 151 ++++++++++-- .../api/src/iceberg_api/web/static/js/tags.js | 64 ++++- .../partials/source_fields_fileshare.html | 128 ++++++++++ .../web/templates/partials/source_form.html | 20 +- .../web/templates/sources/detail.html | 40 +++- .../web/templates/sources/list.html | 2 +- apps/api/tests/test_web_screens.py | 218 ++++++++++++++---- docs/connectors.md | 4 +- docs/web.md | 2 +- 11 files changed, 568 insertions(+), 75 deletions(-) create mode 100644 apps/api/src/iceberg_api/web/templates/partials/source_fields_fileshare.html diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e6da6b..daf3e9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,15 @@ says is recorded beside the finding, never written onto it. Both are reversible. ### Added +- File-share sources are configurable from the console (#196). The create/edit form gained the + share's own fields — protocol, mount path, roots, include/exclude globs, symlink policy and the + per-file ceiling — and `fileshare` joins the type select, so the connector the API has supported + since #145 is no longer API-only to set up. It is deliberately not the Confluence form with + different labels: there is no base URL, no deployment choice, and **no credential box**, because + a share is authenticated by the mount an operator configures on the engine. The details card + describes the share rather than a site, and the connectivity-test button is not offered — a probe + from the API would reach the wrong machine, which is worse than no answer. + - External hand-over (#141, #179–#186): admin-configured targets receive a signed POST with a finding's context, delivered through the same outbox pattern as notifications; the receiver can report its own state back through a signed callback, and an analyst resolves any divergence from diff --git a/apps/api/src/iceberg_api/sources/probe.py b/apps/api/src/iceberg_api/sources/probe.py index afe7a6b..2dac9d3 100644 --- a/apps/api/src/iceberg_api/sources/probe.py +++ b/apps/api/src/iceberg_api/sources/probe.py @@ -55,6 +55,11 @@ SourceType.JIRA: (JiraConnection, JIRA_DEFAULT_API_PREFIX, "/myself"), } +#: The types a connectivity check exists for, for callers that want to ask before +#: offering one — the console hides the button rather than showing one that can +#: only ever answer "not from here" (#196). +PROBEABLE_TYPES = frozenset(_PROBES) + TIMEOUT_SECONDS = 10.0 logger = structlog.get_logger() diff --git a/apps/api/src/iceberg_api/web/routes/sources.py b/apps/api/src/iceberg_api/web/routes/sources.py index ec03a19..3fb880b 100644 --- a/apps/api/src/iceberg_api/web/routes/sources.py +++ b/apps/api/src/iceberg_api/web/routes/sources.py @@ -8,6 +8,7 @@ """ import uuid +from dataclasses import dataclass from typing import Annotated, Any from fastapi import APIRouter, Form, HTTPException, Query, Request, Response @@ -19,9 +20,15 @@ from iceberg_api.scans.routes import list_scans, read_source_coverage from iceberg_api.sources import routes as api from iceberg_api.sources.cursor_routes import invalidate_source_cursors, read_source_cursors +from iceberg_api.sources.probe import PROBEABLE_TYPES from iceberg_api.sources.routes import ProberDep from iceberg_api.sources.schedule_routes import list_schedules -from iceberg_api.sources.schemas import SourceCreate, SourceRead, SourceUpdate +from iceberg_api.sources.schemas import ( + FileshareProtocol, + SourceCreate, + SourceRead, + SourceUpdate, +) from iceberg_api.web.dependencies import ( CurrentViewer, Viewer, @@ -34,11 +41,58 @@ router = APIRouter(include_in_schema=False) -#: The select renders these and nothing else. The API supports more (fileshare, -#: via `SUPPORTED_SOURCE_TYPES`), but the console has no form fields for them -#: yet — offering a choice this form cannot express would post a +#: The select renders these and nothing else. Every type the API supports +#: (`SUPPORTED_SOURCE_TYPES`) now has form fields, so the two agree — a test holds +#: them in step, because offering a choice this form cannot express would post a #: Confluence-shaped blob for something else. -SELECTABLE_TYPES = (SourceType.CONFLUENCE, SourceType.JIRA) +SELECTABLE_TYPES = (SourceType.CONFLUENCE, SourceType.JIRA, SourceType.FILESHARE) + +#: Types with no per-type block: everything they need is on the shared fields. +_HTTP_TYPES = (SourceType.CONFLUENCE, SourceType.JIRA) + + +@dataclass(frozen=True, slots=True) +class _FileshareFields: + """The file-share form's own inputs. + + Grouped rather than added as seven more keywords to :func:`_connection_form`, + which already carries one parameter per field for the two HTTP connectors. + A share has nothing in common with them — no URL, no credential, no comments + or attachments — so its inputs travel together. + """ + + protocol: str + mount_path: str + roots: list[str] + include: list[str] + exclude: list[str] + follow_symlinks: bool + #: Read as text, not `int`, so a blank or mistyped ceiling re-renders the form + #: with a message instead of earning FastAPI's own 422 before this route runs. + max_file_bytes: str + + +def _fileshare_connection(fields: _FileshareFields) -> dict[str, Any]: + """The blob for a mounted share (#145, #196). + + No `base_url` and no credential: the engine walks a **read-only mount**, which + is where the host, the share name and the authentication all live. A form that + offered a token box here would be inviting an admin to store a secret that + nothing reads. + """ + try: + ceiling = int(fields.max_file_bytes.strip() or 0) + except ValueError as exc: + raise ValueError("maximum file size must be a whole number of bytes") from exc + return { + "protocol": fields.protocol, + "mount_path": fields.mount_path.strip(), + "roots": fields.roots, + "include": fields.include, + "exclude": fields.exclude, + "follow_symlinks": fields.follow_symlinks, + "max_file_bytes": ceiling, + } def _connection_form( @@ -54,6 +108,7 @@ def _connection_form( include_personal_spaces: bool, include_history: bool, include_archived_projects: bool, + fileshare: _FileshareFields, ) -> dict[str, Any]: """Assemble a connection blob from the form's flat fields, per source type. @@ -63,11 +118,17 @@ def _connection_form( ``email:token`` auth, so a blank string would read as "configured" (docs/connectors.md § Auth). - Both types' inputs are posted on every save — the form keeps both blocks in the - DOM so switching type does not discard typing — so the fields belonging to the - other type are simply not read here. The API's ``extra="forbid"`` model is the - authority either way. + Every type's inputs are posted on every save — the form keeps all the blocks in + the DOM so switching type does not discard typing — so the fields belonging to + the other types are simply not read here. The API's ``extra="forbid"`` model is + the authority either way. """ + if source_type is SourceType.FILESHARE: + # Returns early because a share shares none of the shared fields: sending + # `base_url` or `include_comments` with it would be rejected by + # `FileshareConnection`, which forbids extras. + return _fileshare_connection(fileshare) + connection: dict[str, Any] = { "base_url": base_url.strip(), "include_comments": include_comments, @@ -80,9 +141,11 @@ def _connection_form( connection["projects"] = projects connection["include_history"] = include_history connection["include_archived_projects"] = include_archived_projects - else: - # Never reached through the select, but a hand-posted type must not - # silently produce a Confluence-shaped blob for something else. + else: # pragma: no cover — every current type is handled; this guards the next + # The select and `SUPPORTED_SOURCE_TYPES` agree today, and a test holds + # them there. This is what a connector added to the API before the console + # catches up hits: a refusal the form can show, rather than a + # Confluence-shaped blob posted for something else. raise ValueError(f"the {source_type.value} connector is not available yet") if email: @@ -98,11 +161,18 @@ def _form_state(source: SourceRead | None, connection: dict[str, Any]) -> dict[s "source": source, "connection": connection, "types": SELECTABLE_TYPES, + "protocols": tuple(FileshareProtocol), "island": { "type": (source.type.value if source else SELECTABLE_TYPES[0].value), "spaces": connection.get("spaces", []), "projects": connection.get("projects", []), "email": connection.get("email", ""), + # Three chip lists rather than one: a root is a subtree to walk, a + # glob is a filter over what is found in it, and mixing them up is + # the mistake this screen exists to make hard. + "roots": connection.get("roots", []), + "include": connection.get("include", []), + "exclude": connection.get("exclude", []), "hasCredential": source.has_credential if source else False, "isNew": source is None, }, @@ -162,6 +232,10 @@ async def source_detail( "schedules": schedules.items, "scans": scans.items, "latest_coverage": latest_coverage, + # A share is reached from an *engine*, through a mount this process + # cannot see; a probe from here would check the wrong machine, so the + # button is not offered rather than always failing (#196). + "probeable": source.type in PROBEABLE_TYPES, "form": _form_state(source, source.connection), }, ) @@ -176,8 +250,11 @@ async def create_source( # one parameter per form field store: SecretStoreDep, name: Annotated[str, Form()], source_type: Annotated[str, Form(alias="type")], - base_url: Annotated[str, Form()], - credential: Annotated[str, Form()], + # Both default to empty because a file-share source has neither: the mount + # carries the host and the authentication, so the form does not render either + # field and the browser posts nothing for them (#145, #196). + base_url: Annotated[str, Form()] = "", + credential: Annotated[str, Form()] = "", email: Annotated[str, Form()] = "", api_prefix: Annotated[str, Form()] = "", spaces: Annotated[list[str], Form()] = [], # noqa: B006 # FastAPI reads the default, never mutates it @@ -187,6 +264,13 @@ async def create_source( # one parameter per form field include_personal_spaces: Annotated[str | None, Form()] = None, include_history: Annotated[str | None, Form()] = None, include_archived_projects: Annotated[str | None, Form()] = None, + protocol: Annotated[str, Form()] = FileshareProtocol.SMB.value, + mount_path: Annotated[str, Form()] = "", + roots: Annotated[list[str], Form()] = [], # noqa: B006 # see spaces + include: Annotated[list[str], Form()] = [], # noqa: B006 # see spaces + exclude: Annotated[list[str], Form()] = [], # noqa: B006 # see spaces + follow_symlinks: Annotated[str | None, Form()] = None, + max_file_bytes: Annotated[str, Form()] = "", enabled: Annotated[str | None, Form()] = None, csrf_token: Annotated[str, Form()] = "", ) -> Response: @@ -204,8 +288,9 @@ async def create_source( # one parameter per form field connection: dict[str, Any] = {} try: # Inside the try: a type the console has no form for (hand-posted, since - # the select offers only SELECTABLE_TYPES) raises ValueError, and that - # must re-render the form with the message — not surface as a 500. + # the select offers only SELECTABLE_TYPES) raises ValueError, as does a + # ceiling that is not a number. Both must re-render the form with the + # message rather than surfacing as a 500. connection = _connection_form( chosen, base_url=base_url, @@ -218,6 +303,15 @@ async def create_source( # one parameter per form field include_personal_spaces=checkbox(include_personal_spaces), include_history=checkbox(include_history), include_archived_projects=checkbox(include_archived_projects), + fileshare=_FileshareFields( + protocol=protocol, + mount_path=mount_path, + roots=string_list(roots), + include=string_list(include), + exclude=string_list(exclude), + follow_symlinks=checkbox(follow_symlinks), + max_file_bytes=max_file_bytes, + ), ) body = SourceCreate( name=name.strip(), @@ -242,7 +336,7 @@ async def update_source( # one parameter per form field db: SessionDep, store: SecretStoreDep, name: Annotated[str, Form()], - base_url: Annotated[str, Form()], + base_url: Annotated[str, Form()] = "", # see create_source credential: Annotated[str, Form()] = "", email: Annotated[str, Form()] = "", api_prefix: Annotated[str, Form()] = "", @@ -253,6 +347,13 @@ async def update_source( # one parameter per form field include_personal_spaces: Annotated[str | None, Form()] = None, include_history: Annotated[str | None, Form()] = None, include_archived_projects: Annotated[str | None, Form()] = None, + protocol: Annotated[str, Form()] = FileshareProtocol.SMB.value, + mount_path: Annotated[str, Form()] = "", + roots: Annotated[list[str], Form()] = [], # noqa: B006 # see spaces + include: Annotated[list[str], Form()] = [], # noqa: B006 # see spaces + exclude: Annotated[list[str], Form()] = [], # noqa: B006 # see spaces + follow_symlinks: Annotated[str | None, Form()] = None, + max_file_bytes: Annotated[str, Form()] = "", enabled: Annotated[str | None, Form()] = None, csrf_token: Annotated[str, Form()] = "", ) -> Response: @@ -265,9 +366,10 @@ async def update_source( # one parameter per form field connection: dict[str, Any] = {} try: - # Inside the try: a stored source of a type this form cannot express - # (e.g. fileshare, created through the API) raises ValueError, and that - # must re-render the form with the message — not surface as a 500. + # Inside the try: a stored source of a type this form cannot express — a + # future connector supported by the API before the console catches up — + # raises ValueError, and that must re-render the form with the message + # rather than surfacing as a 500. connection = _connection_form( source.type, base_url=base_url, @@ -280,6 +382,15 @@ async def update_source( # one parameter per form field include_personal_spaces=checkbox(include_personal_spaces), include_history=checkbox(include_history), include_archived_projects=checkbox(include_archived_projects), + fileshare=_FileshareFields( + protocol=protocol, + mount_path=mount_path, + roots=string_list(roots), + include=string_list(include), + exclude=string_list(exclude), + follow_symlinks=checkbox(follow_symlinks), + max_file_bytes=max_file_bytes, + ), ) changes = SourceUpdate( name=name.strip(), diff --git a/apps/api/src/iceberg_api/web/static/js/tags.js b/apps/api/src/iceberg_api/web/static/js/tags.js index 60682d5..444face 100644 --- a/apps/api/src/iceberg_api/web/static/js/tags.js +++ b/apps/api/src/iceberg_api/web/static/js/tags.js @@ -133,14 +133,19 @@ document.addEventListener('alpine:init', () => { * Server/DC by omitting it, which is a rule nobody should have to know — * the select makes it explicit and the hidden email field carries the * actual API contract; - * - `type` toggles which scope block is shown (#144). Both stay in the DOM, - * so switching back does not discard what was already typed. + * - `type` toggles which scope block is shown (#144, #196). All three stay in + * the DOM, so switching back does not discard what was already typed. * - * The scope keys go to the server as repeated `spaces`/`projects` inputs - * rather than JSON: the route assembles the connection blob, so there is - * exactly one place that knows the shape and the API's validation is the only - * validation that counts. The block that is hidden still posts its inputs; the - * route reads only the ones belonging to the source's type. + * The scope keys go to the server as repeated `spaces`/`projects`/`roots`/ + * `include`/`exclude` inputs rather than JSON: the route assembles the + * connection blob, so there is exactly one place that knows the shape and the + * API's validation is the only validation that counts. The blocks that are + * hidden still post their inputs; the route reads only the ones belonging to + * the source's type. + * + * A file share is the odd one out and stays odd on purpose: no base URL, no + * deployment choice, and **no credential**, because the mount carries all three + * (#145). Its chips are paths rather than keys, so they are not upper-cased. * * The Cloud-vs-DC email rule is identical for both products, which is the * whole point of sharing one credential type across connectors. @@ -153,6 +158,13 @@ document.addEventListener('alpine:init', () => { spaceDraft: '', projects: [], projectDraft: '', + roots: [], + rootDraft: '', + include: [], + includeDraft: '', + exclude: [], + excludeDraft: '', + maxFileBytes: '', rotating: false, hasCredential: false, init() { @@ -160,6 +172,9 @@ document.addEventListener('alpine:init', () => { this.type = typeof data.type === 'string' ? data.type : 'confluence'; this.spaces = Array.isArray(data.spaces) ? data.spaces.slice() : []; this.projects = Array.isArray(data.projects) ? data.projects.slice() : []; + this.roots = Array.isArray(data.roots) ? data.roots.slice() : []; + this.include = Array.isArray(data.include) ? data.include.slice() : []; + this.exclude = Array.isArray(data.exclude) ? data.exclude.slice() : []; this.email = typeof data.email === 'string' ? data.email : ''; this.deployment = this.email ? 'cloud' : (data.isNew ? 'cloud' : 'server'); this.hasCredential = data.hasCredential === true; @@ -188,6 +203,41 @@ document.addEventListener('alpine:init', () => { removeProject(key) { this.projects = this.projects.filter((item) => item !== key); }, + // Paths and globs, not keys: case is significant on the filesystems these + // describe, so unlike a space or project key nothing here is upper-cased. A + // leading slash is trimmed because a root is relative to the mount and the + // API answers 422 for one — better to take the obvious meaning than to teach + // the rule with an error. + addRoot() { + this.pushPath('roots', 'rootDraft'); + }, + removeRoot(path) { + this.roots = this.roots.filter((item) => item !== path); + }, + addInclude() { + this.pushPath('include', 'includeDraft'); + }, + removeInclude(glob) { + this.include = this.include.filter((item) => item !== glob); + }, + addExclude() { + this.pushPath('exclude', 'excludeDraft'); + }, + removeExclude(glob) { + this.exclude = this.exclude.filter((item) => item !== glob); + }, + pushPath(list, draft) { + const value = this[draft].trim().replace(/^\/+/, ''); + if (value && !this[list].includes(value)) this[list].push(value); + this[draft] = ''; + }, + get maxFileSummary() { + const bytes = Number.parseInt(this.maxFileBytes, 10); + if (!Number.isFinite(bytes) || bytes < 1) return 'not a size'; + const mib = bytes / (1024 * 1024); + // Whole MiB is the common case and reads better without a decimal point. + return `${mib >= 1 ? `${Number.isInteger(mib) ? mib : mib.toFixed(1)} MiB` : `${bytes} bytes`}`; + }, startRotation() { this.rotating = true; }, diff --git a/apps/api/src/iceberg_api/web/templates/partials/source_fields_fileshare.html b/apps/api/src/iceberg_api/web/templates/partials/source_fields_fileshare.html new file mode 100644 index 0000000..a44a41d --- /dev/null +++ b/apps/api/src/iceberg_api/web/templates/partials/source_fields_fileshare.html @@ -0,0 +1,128 @@ +{#- File-share scope and policy fields (#145, #196). + + Included unconditionally and toggled with `x-show`, like the Confluence and + Jira blocks, so switching type does not discard what an analyst already typed. + Nothing here carries a static `required`: a hidden required input blocks + submit with no visible cause, so `mount_path` is bound with `:required` + instead and is only required while this block is the one on screen. + + There is deliberately no credential field. The engine walks a read-only + mount, which is where the host, the share name and the authentication all + live — a token box here would invite an admin to store a secret nothing + reads. -#} +
+
+ + +
+ +
+ Roots +
+ + +
+
+ + The whole mount. +
+ + Relative to the mount, no leading /. Each becomes one fetch task, which is what + parallelises a scan — and how you say "finance, not the entire file server". + +
+ +
+
+ Include globs +
+ + +
+
+ + Every file. +
+
+ +
+ Exclude globs +
+ + +
+
+ + Nothing. +
+ Exclude always wins. +
+
+ + + + +
diff --git a/apps/api/src/iceberg_api/web/templates/partials/source_form.html b/apps/api/src/iceberg_api/web/templates/partials/source_form.html index ebbe7b7..9b718d7 100644 --- a/apps/api/src/iceberg_api/web/templates/partials/source_form.html +++ b/apps/api/src/iceberg_api/web/templates/partials/source_form.html @@ -45,9 +45,10 @@ {% endif %} +
+
+ {% include "partials/source_fields_confluence.html" %} {% include "partials/source_fields_jira.html" %} + {% include "partials/source_fields_fileshare.html" %} +
+ + {#- Said rather than left blank: an admin who has configured a Confluence + source will look for the token box, and its absence should read as a + decision instead of a missing field (#145). -#} +

+ No credential. A share is authenticated by the mount, which an operator + configures on the engine — so there is nothing to store here, and nothing this console could + rotate. +