Repository: radiocosmology/alpenhorn

Issue:
Need an ArchiveFileCopy-listing command
The `file list` command (#247) lists `ArchiveFile`s, but I think there's a need for an `ArchiveFileCopy`-listing command as well.

The distinction is: `file list` only ever lists a file at most once, where this command would potentially list each copy of a file.

In #202 I propose `file find` for this.

It would allow listing, say, every known corrupt file, something along the lines of:
```
  $ alpenhorn file find --corrupt
  
  File       Node
  --------   ------
  acq/file1  node1
  acq/file1  node2
  acq/file4  node1
  acq/file8  node5
```

Discussion:

Solution patch:
diff --git a/alpenhorn/cli/acq/create.py b/alpenhorn/cli/acq/create.py
index 7e49b7777..87efb9202 100644
--- a/alpenhorn/cli/acq/create.py
+++ b/alpenhorn/cli/acq/create.py
@@ -3,6 +3,7 @@
 import click
 import peewee as pw
 
+from ...common.util import invalid_import_path
 from ...db import database_proxy, ArchiveAcq
 from ..options import cli_option
 from ..cli import echo
@@ -17,6 +18,11 @@ def create(name):
     another existing Acquisition.
     """
 
+    # Validate
+    rejection_reason = invalid_import_path(name)
+    if rejection_reason:
+        raise click.ClickException(f"invalid name: {rejection_reason}")
+
     with database_proxy.atomic():
         try:
             ArchiveAcq.get(name=name)
diff --git a/alpenhorn/cli/entry.py b/alpenhorn/cli/entry.py
index 20abc72b8..2232cf1a0 100644
--- a/alpenhorn/cli/entry.py
+++ b/alpenhorn/cli/entry.py
@@ -5,7 +5,7 @@
 import click
 
 from .. import db
-from ..common.util import start_alpenhorn, version_option
+from ..common.util import help_config_option, start_alpenhorn, version_option
 
 from . import acq, db, file, group, node
 from .options import not_both
@@ -65,6 +65,7 @@ def _verbosity_from_cli(verbose: int, debug: bool, quiet: int) -> int:
     show_default=False,
     default=False,
 )
+@help_config_option
 def entry(conf, quiet, verbose, debug):
     """Alpenhorn data management system.
 
diff --git a/alpenhorn/cli/file/__init__.py b/alpenhorn/cli/file/__init__.py
index de08aea59..f7a056319 100644
--- a/alpenhorn/cli/file/__init__.py
+++ b/alpenhorn/cli/file/__init__.py
@@ -5,6 +5,7 @@
 
 from .clean import clean
 from .create import create
+from .find import find
 from .import_ import import_
 from .list import list_
 from .modify import modify
@@ -19,6 +20,7 @@ def cli():
 
 cli.add_command(clean, "clean")
 cli.add_command(create, "create")
+cli.add_command(find, "find")
 cli.add_command(import_, "import")
 cli.add_command(list_, "list")
 cli.add_command(modify, "modify")
diff --git a/alpenhorn/cli/file/create.py b/alpenhorn/cli/file/create.py
index d091c76f7..8d0f2eb9d 100644
--- a/alpenhorn/cli/file/create.py
+++ b/alpenhorn/cli/file/create.py
@@ -4,7 +4,7 @@
 import pathlib
 import peewee as pw
 
-from ...common.util import md5sum_file
+from ...common.util import md5sum_file, invalid_import_path
 from ...db import (
     ArchiveAcq,
     ArchiveFile,
@@ -16,7 +16,7 @@
     cli_option,
     not_both,
     requires_other,
-    resolve_acqs,
+    resolve_acq,
     validate_md5,
 )
 
@@ -91,6 +91,11 @@ def create(name, acq_name, from_file, md5, prefix, size):
     if size is not None and size < 0:
         raise click.ClickException(f"negative file size.")
 
+    # Validate
+    rejection_reason = invalid_import_path(name)
+    if rejection_reason:
+        raise click.ClickException(f"invalid name: {rejection_reason}")
+
     validate_md5(md5)
 
     # Scan a file, if requested
@@ -101,7 +106,7 @@ def create(name, acq_name, from_file, md5, prefix, size):
         # database consistency).
         with database_proxy.atomic():
             # Resolve acq
-            acq = resolve_acqs([acq_name])[0]
+            acq = resolve_acq(acq_name)
 
             # Check that "name" isn't already a file in acq
             try:
@@ -144,7 +149,7 @@ def create(name, acq_name, from_file, md5, prefix, size):
     # Create the ArchiveFile
     with database_proxy.atomic():
         # Resolve acq
-        acq = resolve_acqs([acq_name])[0]
+        acq = resolve_acq(acq_name)
 
         # Check that "name" isn't already a file in acq.  We _may_ have already
         # done this once, but we need to do it again inside this transaction.
diff --git a/alpenhorn/cli/file/find.py b/alpenhorn/cli/file/find.py
new file mode 100644
index 000000000..1f5c70821
--- /dev/null
+++ b/alpenhorn/cli/file/find.py
@@ -0,0 +1,91 @@
+"""alpenhorn file list command."""
+
+import click
+import peewee as pw
+from tabulate import tabulate
+
+from ...common.util import pretty_bytes
+from ...db import (
+    ArchiveAcq,
+    ArchiveFile,
+    ArchiveFileCopy,
+    ArchiveFileCopyRequest,
+    StorageGroup,
+    StorageNode,
+)
+from ..cli import echo, pretty_time
+from ..options import (
+    cli_option,
+    not_both,
+    both_or_neither,
+    files_in_nodes,
+    files_in_groups,
+    resolve_acq,
+    resolve_group,
+    resolve_node,
+    state_constraint,
+)
+
+
+@click.command()
+@cli_option("acq")
+@click.option("--corrupt", is_flag=True, help="Find corrupt files.")
+@click.option(
+    "--group",
+    metavar="GROUP",
+    multiple=True,
+    help="May be specified multiple times.  Limit search to GROUP(s).",
+)
+@click.option("--healthy", is_flag=True, help="Find healthy files.")
+@click.option("--missing", is_flag=True, help="Find missing files.")
+@click.option(
+    "--node",
+    metavar="NODE",
+    multiple=True,
+    help="May be specified multiple times.  Limit search to NODE(s).",
+)
+@click.option("--suspect", is_flag=True, help="Find suspect files.")
+@click.pass_context
+def find(ctx, acq, corrupt, group, healthy, missing, node, suspect):
+    """Find Files on Nodes.
+
+    Without options, lists every healthy File on every Node in the
+    Data Index.  Options can be used to change and limit the list.
+    """
+
+    nodes = resolve_node(node)
+    groups = resolve_group(group)
+    acqs = resolve_acq(acq)
+
+    state_expr = state_constraint(
+        corrupt=corrupt, healthy=healthy, missing=missing, suspect=suspect
+    )
+    if state_expr is None:
+        state_expr = state_constraint(healthy=True)
+
+    # Query
+    query = ArchiveFileCopy.select().join(ArchiveFile).where(state_expr)
+
+    # Add limits, when given
+    if acqs:
+        query = query.where(ArchiveFile.acq << acqs)
+    if groups:
+        query = query.switch(ArchiveFileCopy).join(StorageNode)
+        # Groups and Nodes need to be or'd together if both provided
+        if nodes:
+            query = query.where(
+                (ArchiveFileCopy.node << nodes) | (StorageNode.group << groups)
+            )
+        else:
+            query = query.where(StorageNode.group << groups)
+    elif nodes:
+        query = query.where(ArchiveFileCopy.node << nodes)
+
+    data = []
+    for copy in query.execute():
+        data.append((copy.file.path, copy.node.name, copy.state))
+
+    if data:
+        echo(tabulate(sorted(data), headers=["File", "Node", "State"]))
+    else:
+        echo("No match.")
diff --git a/alpenhorn/cli/file/list.py b/alpenhorn/cli/file/list.py
index 28598d9c9..64e02fc19 100644
--- a/alpenhorn/cli/file/list.py
+++ b/alpenhorn/cli/file/list.py
@@ -20,7 +20,7 @@
     both_or_neither,
     files_in_nodes,
     files_in_groups,
-    resolve_acqs,
+    resolve_acq,
     resolve_group,
     resolve_node,
     state_constraint,
@@ -221,9 +221,9 @@ def list_(
     detail_group = None
     if details and not from_:
         if node and not group and len(nodes) == 1:
-            detail_node = nodes[0]
+            detail_node = nodes.pop()
         elif group and not node and len(groups) == 1:
-            detail_group = groups[0]
+            detail_group = groups.pop()
 
     # The negative selection list
     if absent_node and absent_group:
@@ -300,7 +300,7 @@ def list_(
 
     # Apply the --acq limit, if given
     if acq:
-        query = query.where(ArchiveFile.acq << resolve_acqs(acq))
+        query = query.where(ArchiveFile.acq << resolve_acq(acq))
 
     # Apply syncability, if present
     if syncable_files is not None:
diff --git a/alpenhorn/cli/group/sync.py b/alpenhorn/cli/group/sync.py
index 3f7aee2db..5cad9fbf9 100644
--- a/alpenhorn/cli/group/sync.py
+++ b/alpenhorn/cli/group/sync.py
@@ -22,7 +22,7 @@
     files_in_groups,
     not_both,
     requires_other,
-    resolve_acqs,
+    resolve_acq,
     resolve_group,
 )
 
@@ -383,7 +383,7 @@ def run_query(
             raise click.ClickException("No such group: " + node_name)
 
     # Resolve acqs
-    acqs = resolve_acqs(acq)
+    acqs = resolve_acq(acq)
 
     # Cancel and sync are different enough that they've been broken up into two functions:
     if cancel:
diff --git a/alpenhorn/cli/node/clean.py b/alpenhorn/cli/node/clean.py
index af1a23722..084cb0d65 100644
--- a/alpenhorn/cli/node/clean.py
+++ b/alpenhorn/cli/node/clean.py
@@ -19,7 +19,7 @@
     cli_option,
     files_in_groups,
     not_both,
-    resolve_acqs,
+    resolve_acq,
     resolve_node,
     resolve_group,
 )
@@ -97,7 +97,7 @@ def _run_query(
             ctx.exit()
 
         # Resolve acqs
-        acqs = resolve_acqs(acq)
+        acqs = resolve_acq(acq)
 
         # Find all candidate file copies
         query = ArchiveFileCopy.select().where(ArchiveFileCopy.node == node, has_file)
diff --git a/alpenhorn/cli/node/verify.py b/alpenhorn/cli/node/verify.py
index c0ec24e22..e91f66fd4 100644
--- a/alpenhorn/cli/node/verify.py
+++ b/alpenhorn/cli/node/verify.py
@@ -7,7 +7,7 @@
 
 from ...common.util import pretty_bytes
 from ...db import ArchiveAcq, ArchiveFile, ArchiveFileCopy, database_proxy
-from ..options import cli_option, not_both, resolve_acqs, resolve_node, state_constraint
+from ..options import cli_option, not_both, resolve_acq, resolve_node, state_constraint
 from ..cli import check_then_update, echo
 
 
@@ -48,7 +48,7 @@ def _run_query(
         node = resolve_node(name)
 
         # Resolve acqs
-        acqs = resolve_acqs(acq)
+        acqs = resolve_acq(acq)
 
         # Find all candidate file copies
         query = ArchiveFileCopy.select(ArchiveFileCopy.id).where(
diff --git a/alpenhorn/cli/options.py b/alpenhorn/cli/options.py
index 144d1e9c4..5d8ead45f 100644
--- a/alpenhorn/cli/options.py
+++ b/alpenhorn/cli/options.py
@@ -262,12 +262,12 @@ def requires_other(
         raise click.UsageError(f"--{opt1_name} may only be used with --{opt2_name}")
 
 
-def resolve_group(group: str | list[str]) -> StorageGroup | list[StorageGroup]:
+def resolve_group(group: str | list[str]) -> StorageGroup | set[StorageGroup]:
     """Convert group name(s) into StorageGroup(s).
 
     If given a single `str`, returns a single `StorageGroup`.
     Otherwise, should be given a list of str and will return a
-    list of StorageGroups.
+    set of StorageGroups.
 
     If any name can't be resolved, raises ClickException.
     """
@@ -275,24 +275,24 @@ def resolve_group(group: str | list[str]) -> StorageGroup | list[StorageGroup]:
     if one_group:
         group = [group]
 
-    groups = []
+    groups = set()
     for name in group:
         try:
-            groups.append(StorageGroup.get(name=name))
+            groups.add(StorageGroup.get(name=name))
         except pw.DoesNotExist:
             raise click.ClickException("no such group: " + name)
 
     if one_group:
-        return groups[0]
+        return groups.pop()
     return groups
 
 
-def resolve_node(node: str | list[str]) -> StorageNode | list[StorageNode]:
+def resolve_node(node: str | list[str]) -> StorageNode | set[StorageNode]:
     """Convert node name(s) into StorageNode(s).
 
     If given a single `str`, returns a single `StorageNode`.
     Otherwise, should be given a list of str and will return a
-    list of StorageNodes.
+    set of StorageNodes.
 
     If any name can't be resolved, raises ClickException.
     """
@@ -300,32 +300,42 @@ def resolve_node(node: str | list[str]) -> StorageNode | list[StorageNode]:
     if one_node:
         node = [node]
 
-    nodes = []
+    nodes = set()
     for name in node:
         try:
-            nodes.append(StorageNode.get(name=name))
+            nodes.add(StorageNode.get(name=name))
         except pw.DoesNotExist:
             raise click.ClickException("no such node: " + name)
 
     if one_node:
-        return nodes[0]
+        return nodes.pop()
     return nodes
 
 
-def resolve_acqs(acq: list[str]) -> list[ArchiveAcq]:
+def resolve_acq(acq: str | list[str]) -> ArchiveAcq | set[ArchiveAcq]:
     """Convert --acq list to ArchiveAcq list.
 
-    If the input list is empty, so is the output list.
+    If given a single `str`, returns a single `ArchiveAcq`.
+    Otherwise, should be given a list of str and will return a
+    set of ArchiveAcqs.
 
     Raises `click.ClickException` if a non-existent acqusition was
     provided.
     """
-    acqs = []
+
+    one_acq = isinstance(acq, str)
+    if one_acq:
+        acq = [acq]
+
+    acqs = set()
     for acqname in acq:
         try:
-            acqs.append(ArchiveAcq.get(name=acqname))
+            acqs.add(ArchiveAcq.get(name=acqname))
         except pw.DoesNotExist:
             raise click.ClickException("No such acquisition: " + acqname)
+
+    if one_acq:
+        return acqs.pop()
     return acqs
 
 
diff --git a/alpenhorn/common/config.py b/alpenhorn/common/config.py
index 4adf4ce99..e5c6f05c9 100644
--- a/alpenhorn/common/config.py
+++ b/alpenhorn/common/config.py
@@ -182,7 +182,7 @@ def load_config(cli_conf: os.PathLike, cli: bool) -> None:
     if cli_conf:
         config_files.append(cli_conf)
 
-    any_exist = False
+    no_config = True
 
     for cfile in config_files:
         # Expand the configuration file path
@@ -198,7 +198,7 @@ def load_config(cli_conf: os.PathLike, cli: bool) -> None:
                 )
             continue
 
-        any_exist = True
+        no_config = False
 
         log.info("Loading config file %s", cfile)
 
@@ -208,13 +208,10 @@ def load_config(cli_conf: os.PathLike, cli: bool) -> None:
         if conf is not None:
             config = merge_dict_tree(config, conf)
 
-    if not any_exist:
-        if cli:
-            exc = ClickException
-        else:
-            exc = RuntimeError
-
-        raise exc("No configuration files available.")
+    if no_config:
+        raise ClickException(
+            "No configuration files available.  See --help-config for more details."
+        )
 
 
 def merge_dict_tree(a: dict, b: dict) -> dict:
diff --git a/alpenhorn/common/util.py b/alpenhorn/common/util.py
index 905b75285..625719dee 100644
--- a/alpenhorn/common/util.py
+++ b/alpenhorn/common/util.py
@@ -14,6 +14,62 @@
 log = logging.getLogger(__name__)
 
 
+def help_config_option(func):
+    """Click --help-config option"""
+
+    # This is the callback
+    def _help_config(ctx, param, value):
+        if not value or ctx.resilient_parsing:
+            return
+
+        click.echo(
+            """
+In order to operate, alpenhorn needs to be configured to point it to the
+SQL database containing its Data Index.  This is done through one or more
+alpenhorn config files.
+
+Alpenhorn searches for config files in the following order:
+\b
+  * /etc/alpenhorn/alpenhorn.conf
+  * /etc/xdg/alpenhorn/alpenhorn.conf
+  * ~/.config/alpenhorn/alpenhorn.conf
+  * the value of the "ALPENHORN_CONFIG_FILE" environment variable
+  * the path passed via "-c" or "--conf" on the command line
+
+If multiple config files from this list are found, all will be read,
+with config from later files overriding earlier ones.  The config files
+are YAML.
+
+Typically, to configure the database connection, the config should define
+a database URL, with a YAML config like this:
+\b
+database:
+	url: mysql://user:passwd@hostname:port/my_database
+
+However, an alternate way to provide database configuration to alpenhorn
+is through a database extension module.  If you use such an extension, you
+should declare it in the config, instead, so alpenhorn can load it:
+\b
+extensions:
+	- my_extensions.alpenhorn_db_extension
+
+The database connection is the only thing that can be configured for the
+alpenhorn CLI.  But further configuration of the daemon is possible.
+Consult the alpenhorn documentation for more information.
+"""
+        )
+        ctx.exit(0)
+
+    return click.option(
+        "--help-config",
+        is_flag=True,
+        callback=_help_config,
+        expose_value=False,
+        is_eager=True,
+        help="Help on configuring alpenhorn.",
+    )(func)
+
+
 def version_option(func):
     """Click --version option"""
     return click.option(
@@ -313,3 +369,47 @@ def pretty_deltat(seconds: float) -> str:
 
     # For short durations, include tenths of a second
     return f"{seconds:.1f}s"
+
+
+def invalid_import_path(name: str) -> str | None:
+    """Is `name` invalid as an import path?
+
+    i.e., can `name` be used as an ArchiveAcq or
+    ArchiveFile name (or both, combined).
+
+    Returns
+    -------
+    rejection_reason: str or None
+        A string describing why `name` was rejected,
+        or None, if the name was valid.
+    """
+
+    # Can't be the null string
+    if name == "":
+        return "empty path"
+
+    # Can't simply be "." or ".."
+    if name == "." or name == "..":
+        return "invalid path"
+
+    # Can't start with "/" or "./" or "../"
+    if name.startswith("/") or name.startswith("./") or name.startswith("../"):
+        return "invalid start"
+
+    # Can't end with "/" or "/." or "/.."
+    if name.endswith("/") or name.endswith("/.") or name.endswith("/.."):
+        return "invalid end"
+
+    # Can't have multiple "/" in a row
+    if "//" in name:
+        return "repeated /"
+
+    # Can't have internal "/./"
+    if "/./" in name:
+        return 'invalid path element "."'
+
+    # Can't have internal "/../"
+    if "/../" in name:
+        return 'invalid path element ".."'
+
+    return None
diff --git a/alpenhorn/daemon/auto_import.py b/alpenhorn/daemon/auto_import.py
index a25852d1f..224132424 100644
--- a/alpenhorn/daemon/auto_import.py
+++ b/alpenhorn/daemon/auto_import.py
@@ -15,6 +15,7 @@
 
 from .. import db
 from ..common import config, extensions
+from ..common.util import invalid_import_path
 from ..db import (
     ArchiveAcq,
     ArchiveFile,
@@ -173,6 +174,15 @@ def _import_file(
             req.complete()
         return
 
+    # Vet acq_name from extension
+    rejection_reason = invalid_import_path(str(acq_name))
+    if rejection_reason:
+        log.warning(f'Rejecting invalid acq path "{acq_name}": {rejection_reason}')
+        if req:
+            log.info(f"Completed import request #{req.id}.")
+            req.complete()
+        return
+
     file_name = path.relative_to(acq_name)
 
     # If a copy already exists, we're done
diff --git a/alpenhorn/daemon/entry.py b/alpenhorn/daemon/entry.py
index 5910c93cd..f46b24665 100755
--- a/alpenhorn/daemon/entry.py
+++ b/alpenhorn/daemon/entry.py
@@ -6,7 +6,7 @@
 
 from .. import db
 from ..common import config
-from ..common.util import start_alpenhorn, version_option
+from ..common.util import help_config_option, start_alpenhorn, version_option
 from ..scheduler import FairMultiFIFOQueue, pool
 from . import auto_import, update
 
@@ -33,6 +33,7 @@ def log_exception(*args):
     metavar="FILE",
 )
 @version_option
+@help_config_option
 def entry(conf):
     """Alpenhorn data management daemon.
 
diff --git a/alpenhorn/daemon/update.py b/alpenhorn/daemon/update.py
index 08c2aede5..9d71d6bc9 100644
--- a/alpenhorn/daemon/update.py
+++ b/alpenhorn/daemon/update.py
@@ -491,7 +491,29 @@ def update_import(self) -> None:
                 continue
 
             if req.recurse:
-                # If recursion was requested, run a scan on the path
+                # Ensure the base directory we want to scan is in-tree
+                fullpath = pathlib.Path(self.db.root, path)
+                try:
+                    fullpath = fullpath.resolve(strict=True)
+                except OSError as e:
+                    log.warning(
+                        f"Ignoring import request of unresolvable scan path: {fullpath}"
+                    )
+                    req.complete()
+                    continue
+
+                # Recompute the relative path after resolution, or skip scan if we're
+                # now out-of-tree
+                try:
+                    path = fullpath.relative_to(self.db.root)
+                except ValueError:
+                    log.warning(
+                        f"Ignoring import request of out-of-tree scan path: {fullpath}"
+                    )
+                    req.complete()
+                    continue
+
+                # Run scan
                 Task(
                     func=auto_import.scan,
                     queue=self._queue,
@@ -500,7 +522,17 @@ def update_import(self) -> None:
                     name=f'Scan "{path}" on {self.name}',
                 )
             else:
-                # Otherwise, try to directly import the path
+                # Check that the import path is valid
+                rejection_reason = util.invalid_import_path(req.path)
+                if rejection_reason:
+                    log.warning(
+                        f'Ignoring request for import of invalid path "{req.path}": '
+                        + rejection_reason
+                    )
+                    req.complete()
+                    continue
+
+                # Try to directly import the path
                 auto_import.import_file(self, self._queue, req.path, req.register, req)
 
     def update(self) -> None:


Test patch:
diff --git a/tests/cli/acq/test_create.py b/tests/cli/acq/test_create.py
index 3794d594d..ad50a09f6 100644
--- a/tests/cli/acq/test_create.py
+++ b/tests/cli/acq/test_create.py
@@ -14,6 +14,15 @@ def test_create(clidb, cli):
     ArchiveAcq.get(name="TEST")
 
 
+def test_bad_name(clidb, cli):
+    """Test rejection of invalid name."""
+
+    cli(1, ["acq", "create", "/test/"])
+
+    # No acq was created
+    assert ArchiveAcq.select().count() == 0
+
+
 def test_create_existing(clidb, cli):
     """Test creating an acq that already exists."""
 
diff --git a/tests/cli/file/test_create.py b/tests/cli/file/test_create.py
index cc179bf33..e62576479 100644
--- a/tests/cli/file/test_create.py
+++ b/tests/cli/file/test_create.py
@@ -16,6 +16,24 @@ def test_no_data(clidb, cli):
     cli(2, ["file", "create", "name", "Acq", "--size=3"])
 
 
+def test_bad_name(clidb, cli):
+    """Test an invalid file name."""
+
+    ArchiveAcq.create(name="Acq")
+
+    cli(
+        1,
+        [
+            "file",
+            "create",
+            "name/../name",
+            "Acq",
+            "--md5=0123456789ABCDEF0123456789ABCDEF",
+            "--size=3",
+        ],
+    )
+
+
 def test_negative_size(clidb, cli):
     """Test a negative size."""
 
diff --git a/tests/cli/file/test_find.py b/tests/cli/file/test_find.py
new file mode 100644
index 000000000..49444699b
--- /dev/null
+++ b/tests/cli/file/test_find.py
@@ -0,0 +1,248 @@
+"""Test CLI: alpenhorn file find"""
+
+from alpenhorn.db import (
+    StorageGroup,
+    StorageNode,
+    ArchiveAcq,
+    ArchiveFile,
+    ArchiveFileCopy,
+)
+
+
+def test_bad_acq(clidb, cli):
+    """Test a non-existent --acq"""
+
+    cli(1, ["file", "find", "--acq=MISSING"])
+
+
+def test_bad_node(clidb, cli):
+    """Test a non-existent --node"""
+
+    cli(1, ["file", "find", "--node=MISSING"])
+
+
+def test_bad_group(clidb, cli):
+    """Test a non-existent --group"""
+
+    cli(1, ["file", "find", "--group=MISSING"])
+
+
+def test_everything(clidb, cli, assert_row_present):
+    """Test listing everything."""
+
+    group = StorageGroup.create(name="Group")
+    node1 = StorageNode.create(name="Node1", group=group)
+    node2 = StorageNode.create(name="Node2", group=group)
+
+    acq = ArchiveAcq.create(name="acq1")
+    file = ArchiveFile.create(name="file1", acq=acq)
+    ArchiveFileCopy.create(file=file, node=node1, has_file="Y", wants_file="Y")
+    ArchiveFileCopy.create(file=file, node=node2, has_file="X", wants_file="Y")
+    file = ArchiveFile.create(name="file2", acq=acq)
+    ArchiveFileCopy.create(file=file, node=node1, has_file="M", wants_file="Y")
+    ArchiveFileCopy.create(file=file, node=node2, has_file="N", wants_file="Y")
+    acq = ArchiveAcq.create(name="acq2")
+    file = ArchiveFile.create(name="file3", acq=acq)
+    ArchiveFileCopy.create(file=file, node=node1, has_file="Y", wants_file="M")
+    ArchiveFileCopy.create(file=file, node=node2, has_file="Y", wants_file="Y")
+
+    result = cli(0, ["file", "find"])
+
+    assert_row_present(result.output, "acq1/file1", "Node1", "Present")
+    assert_row_present(result.output, "acq2/file3", "Node1", "Removable")
+    assert_row_present(result.output, "acq2/file3", "Node2", "Present")
+    assert result.output.count("acq") == 3
+
+
+def test_acq(clidb, cli, assert_row_present):
+    """Test limiting by --acq."""
+
+    group = StorageGroup.create(name="Group")
+    node = StorageNode.create(name="Node", group=group)
+
+    acq = ArchiveAcq.create(name="acq1")
+    file = ArchiveFile.create(name="file", acq=acq)
+    ArchiveFileCopy.create(file=file, node=node, has_file="Y", wants_file="Y")
+    acq = ArchiveAcq.create(name="acq2")
+    file = ArchiveFile.create(name="file", acq=acq)
+    ArchiveFileCopy.create(file=file, node=node, has_file="Y", wants_file="Y")
+
+    result = cli(0, ["file", "find", "--acq=acq1"])
+
+    assert_row_present(result.output, "acq1/file", "Node", "Present")
+    assert result.output.count("acq") == 1
+
+
+def test_node(clidb, cli, assert_row_present):
+    """Test limiting by --node."""
+
+    group = StorageGroup.create(name="Group")
+    node1 = StorageNode.create(name="Node1", group=group)
+    node2 = StorageNode.create(name="Node2", group=group)
+    node3 = StorageNode.create(name="Node3", group=group)
+
+    acq = ArchiveAcq.create(name="acq")
+    file = ArchiveFile.create(name="file1", acq=acq)
+    ArchiveFileCopy.create(file=file, node=node1, has_file="Y", wants_file="Y")
+    ArchiveFileCopy.create(file=file, node=node2, has_file="Y", wants_file="Y")
+
+    file = ArchiveFile.create(name="file2", acq=acq)
+    ArchiveFileCopy.create(file=file, node=node1, has_file="Y", wants_file="Y")
+    ArchiveFileCopy.create(file=file, node=node2, has_file="N", wants_file="N")
+
+    file = ArchiveFile.create(name="file3", acq=acq)
+    ArchiveFileCopy.create(file=file, node=node2, has_file="Y", wants_file="Y")
+    ArchiveFileCopy.create(file=file, node=node3, has_file="Y", wants_file="Y")
+
+    file = ArchiveFile.create(name="file4", acq=acq)
+    ArchiveFileCopy.create(file=file, node=node1, has_file="X", wants_file="Y")
+    ArchiveFileCopy.create(file=file, node=node3, has_file="Y", wants_file="Y")
+
+    result = cli(0, ["file", "find", "--node=Node1", "--node=Node2"])
+
+    assert_row_present(result.output, "acq/file1", "Node1", "Present")
+    assert_row_present(result.output, "acq/file1", "Node2", "Present")
+    assert_row_present(result.output, "acq/file2", "Node1", "Present")
+    assert_row_present(result.output, "acq/file3", "Node2", "Present")
+    assert result.output.count("acq") == 4
+
+
+def test_group(clidb, cli, assert_row_present):
+    """Test liming by --group."""
+
+    group1 = StorageGroup.create(name="Group1")
+    node1 = StorageNode.create(name="Node1", group=group1)
+
+    group2 = StorageGroup.create(name="Group2")
+    node2 = StorageNode.create(name="Node2", group=group2)
+
+    group3 = StorageGroup.create(name="Group3")
+    node3 = StorageNode.create(name="Node3", group=group3)
+
+    acq = ArchiveAcq.create(name="acq")
+    file = ArchiveFile.create(name="file1", acq=acq)
+    ArchiveFileCopy.create(file=file, node=node1, has_file="Y", wants_file="Y")
+    ArchiveFileCopy.create(file=file, node=node2, has_file="Y", wants_file="Y")
+
+    file = ArchiveFile.create(name="file2", acq=acq)
+    ArchiveFileCopy.create(file=file, node=node1, has_file="Y", wants_file="Y")
+    ArchiveFileCopy.create(file=file, node=node2, has_file="N", wants_file="N")
+
+    file = ArchiveFile.create(name="file3", acq=acq)
+    ArchiveFileCopy.create(file=file, node=node2, has_file="Y", wants_file="Y")
+    ArchiveFileCopy.create(file=file, node=node3, has_file="Y", wants_file="Y")
+
+    file = ArchiveFile.create(name="file4", acq=acq)
+    ArchiveFileCopy.create(file=file, node=node1, has_file="X", wants_file="Y")
+    ArchiveFileCopy.create(file=file, node=node3, has_file="Y", wants_file="Y")
+
+    result = cli(0, ["file", "find", "--group=Group1", "--group=Group2"])
+
+    assert_row_present(result.output, "acq/file1", "Node1", "Present")
+    assert_row_present(result.output, "acq/file1", "Node2", "Present")
+    assert_row_present(result.output, "acq/file2", "Node1", "Present")
+    assert_row_present(result.output, "acq/file3", "Node2", "Present")
+    assert result.output.count("acq") == 4
+
+
+def test_node_group(clidb, cli, assert_row_present):
+    """Test --node and --group together."""
+
+    group1 = StorageGroup.create(name="Group1")
+    node1 = StorageNode.create(name="Node1", group=group1)
+
+    group2 = StorageGroup.create(name="Group2")
+    node2 = StorageNode.create(name="Node2", group=group2)
+
+    group3 = StorageGroup.create(name="Group3")
+    node3 = StorageNode.create(name="Node3", group=group3)
+
+    acq = ArchiveAcq.create(name="acq")
+    file = ArchiveFile.create(name="file1", acq=acq)
+    ArchiveFileCopy.create(file=file, node=node1, has_file="Y", wants_file="Y")
+    ArchiveFileCopy.create(file=file, node=node2, has_file="Y", wants_file="Y")
+
+    file = ArchiveFile.create(name="file2", acq=acq)
+    ArchiveFileCopy.create(file=file, node=node1, has_file="Y", wants_file="Y")
+    ArchiveFileCopy.create(file=file, node=node2, has_file="N", wants_file="N")
+
+    file = ArchiveFile.create(name="file3", acq=acq)
+    ArchiveFileCopy.create(file=file, node=node2, has_file="Y", wants_file="Y")
+    ArchiveFileCopy.create(file=file, node=node3, has_file="Y", wants_file="Y")
+
+    file = ArchiveFile.create(name="file4", acq=acq)
+    ArchiveFileCopy.create(file=file, node=node1, has_file="X", wants_file="Y")
+    ArchiveFileCopy.create(file=file, node=node3, has_file="Y", wants_file="Y")
+
+    result = cli(0, ["file", "find", "--group=Group1", "--node=Node1", "--node=Node2"])
+
+    assert_row_present(result.output, "acq/file1", "Node1", "Present")
+    assert_row_present(result.output, "acq/file1", "Node2", "Present")
+    assert_row_present(result.output, "acq/file2", "Node1", "Present")
+    assert_row_present(result.output, "acq/file3", "Node2", "Present")
+    assert result.output.count("acq") == 4
+
+
+def test_state(clidb, cli, assert_row_present):
+    """Test the state flags."""
+
+    acq = ArchiveAcq.create(name="acq")
+    group = StorageGroup.create(name="Group")
+    node = StorageNode.create(name="Node", group=group)
+
+    for has in ["Y", "M", "X", "N"]:
+        for wants in ["Y", "M", "N"]:
+            file = ArchiveFile.create(name="File" + has + wants, acq=acq)
+            ArchiveFileCopy.create(file=file, node=node, has_file=has, wants_file=wants)
+
+    # The rows we want to find for each state
+    state_rows = {
+        "corrupt": [
+            ("acq/FileXY", "Node", "Corrupt"),
+            ("acq/FileXM", "Node", "Corrupt"),
+        ],
+        "healthy": [
+            ("acq/FileYY", "Node", "Present"),
+            ("acq/FileYM", "Node", "Removable"),
+        ],
+        "suspect": [
+            ("acq/FileMY", "Node", "Suspect"),
+            ("acq/FileMM", "Node", "Suspect"),
+        ],
+        "missing": [("acq/FileNY", "Node", "Missing")],
+    }
+
+    # Run through all the state combinations.  There are 2**4 of these:
+    for num in range(16):
+        corrupt = num & 1 == 0
+        healthy = num & 2 == 0
+        suspect = num & 4 == 0
+        missing = num & 8 == 0
+
+        # Build command and resultant file list
+        command = ["file", "find"]
+        rows = []
+        if corrupt:
+            command.append("--corrupt")
+            rows += state_rows["corrupt"]
+        if suspect:
+            command.append("--suspect")
+            rows += state_rows["suspect"]
+        if missing:
+            command.append("--missing")
+            rows += state_rows["missing"]
+        if healthy:
+            command.append("--healthy")
+            rows += state_rows["healthy"]
+        # The default when no state is chosen
+        if not rows:
+            rows = state_rows["healthy"]
+
+        # Execute
+        result = cli(0, command)
+
+        # Check files
+        for row in rows:
+            assert_row_present(result.output, *row)
+        # Count rows to make sure there aren't extras
+        assert result.output.count("acq") == len(rows)
diff --git a/tests/common/test_config.py b/tests/common/test_config.py
index b3a5f2095..d145c0610 100644
--- a/tests/common/test_config.py
+++ b/tests/common/test_config.py
@@ -1,6 +1,8 @@
-import os
+"""Test common.config"""
 
+import os
 import pytest
+from click import ClickException
 
 from alpenhorn.common import config
 
@@ -15,7 +17,7 @@ def merge_dict(a, b):
 def test_no_config():
     # Check that alpenhorn fails if it has no appropriate configuration
 
-    with pytest.raises(RuntimeError) as excinfo:
+    with pytest.raises(ClickException) as excinfo:
         config.load_config(None, False)
 
     assert "No configuration" in str(excinfo.value)
diff --git a/tests/common/test_util.py b/tests/common/test_util.py
index 77f3817ca..697ad62a9 100644
--- a/tests/common/test_util.py
+++ b/tests/common/test_util.py
@@ -127,3 +127,33 @@ def test_pretty_deltat():
     assert util.pretty_deltat(0.01) == "0.0s"
     assert util.pretty_deltat(0) == "0.0s"
     assert util.pretty_deltat(-1) == "-1.0s"
+
+
+def test_invalid_import_path():
+    """Test invalid_import_path"""
+
+    # Explicitly forbidden names
+    assert util.invalid_import_path("") is not None
+    assert util.invalid_import_path(".") is not None
+    assert util.invalid_import_path("..") is not None
+
+    # Forbidden starts
+    assert util.invalid_import_path("/name") is not None
+    assert util.invalid_import_path("./name") is not None
+    assert util.invalid_import_path("../name") is not None
+
+    # Forbidden ends
+    assert util.invalid_import_path("name/") is not None
+    assert util.invalid_import_path("name/.") is not None
+    assert util.invalid_import_path("name/..") is not None
+
+    # Forbidden middles
+    assert util.invalid_import_path("name//name") is not None
+    assert util.invalid_import_path("name///name") is not None
+    assert util.invalid_import_path("name/./name") is not None
+    assert util.invalid_import_path("name/../name") is not None
+
+    # These are fine
+    assert util.invalid_import_path("name") is None
+    assert util.invalid_import_path("name/name") is None
+    assert util.invalid_import_path("name/.../name") is None
diff --git a/tests/daemon/test_auto_import.py b/tests/daemon/test_auto_import.py
index 9e2a158e6..676e9d01c 100644
--- a/tests/daemon/test_auto_import.py
+++ b/tests/daemon/test_auto_import.py
@@ -104,6 +104,24 @@ def test_import_file_no_detect(dbtables, unode):
         ArchiveAcq.get(name="acq")
 
 
+def test_import_file_invalid_acqname(dbtables, unode):
+    """Test invalid acq_name from import_detect."""
+
+    with patch(
+        "alpenhorn.common.extensions._id_ext", [lambda path, node: ("acq/", None)]
+    ):
+        with pytest.raises(StopIteration):
+            next(
+                auto_import._import_file(
+                    None, unode, pathlib.PurePath("acq/file"), True, None
+                )
+            )
+
+    # No acq has been added
+    with pytest.raises(pw.DoesNotExist):
+        ArchiveAcq.get(name="acq")
+
+
 def test_import_file_locked(xfs, dbtables, unode):
     """Test bad file in _import_file()"""
 
diff --git a/tests/daemon/test_update_node.py b/tests/daemon/test_update_node.py
index 7c0704d8a..2cd3db15f 100644
--- a/tests/daemon/test_update_node.py
+++ b/tests/daemon/test_update_node.py
@@ -514,6 +514,52 @@ def test_update_import_absolute(unode, queue, archivefileimportrequest):
     assert ArchiveFileCopy.select().count() == 0
 
 
+def test_update_import_invalid_path(unode, queue, archivefileimportrequest):
+    """update_import() should ignore invalid paths."""
+
+    afir = archivefileimportrequest(path="./path/..", node=unode.db)
+
+    unode.update_import()
+
+    # Nothing queued
+    assert queue.qsize == 0
+
+    # Request is completed
+    assert ArchiveFileImportRequest.get(id=afir.id).completed == 1
+
+
+def test_update_import_outtree_scan(unode, xfs, queue, archivefileimportrequest):
+    """Test rejection of out-of-tree scan paths from import requests"""
+
+    xfs.create_dir("/node/import")
+
+    afir = archivefileimportrequest(path="..", node=unode.db, recurse=True)
+
+    unode.update_import()
+
+    # Nothing queued
+    assert queue.qsize == 0
+
+    # Request is completed
+    assert ArchiveFileImportRequest.get(id=afir.id).completed == 1
+
+
+def test_update_import_unresolve_scan(unode, xfs, queue, archivefileimportrequest):
+    """Test rejection of unresolvable scan paths from import requests"""
+
+    xfs.create_dir("/node/import")
+
+    afir = archivefileimportrequest(path="path/subpath/..", node=unode.db, recurse=True)
+
+    unode.update_import()
+
+    # Nothing queued
+    assert queue.qsize == 0
+
+    # Request is completed
+    assert ArchiveFileImportRequest.get(id=afir.id).completed == 1
+
+
 def test_update_import_no_recurse(unode, queue, archivefileimportrequest):
     """Test update_import() with a non-recursive import request."""
 
@@ -532,9 +578,11 @@ def test_update_import_no_recurse(unode, queue, archivefileimportrequest):
     mock.assert_called_with(unode, queue, "import/path", True, afir)
 
 
-def test_update_import_scan(unode, queue, archivefileimportrequest):
+def test_update_import_scan(unode, xfs, queue, archivefileimportrequest):
     """Test update_import() with a recursive import request."""
 
+    xfs.create_dir("/node/import/path")
+
     afir = archivefileimportrequest(
         path="import/path", node=unode.db, recurse=True, register=True
     )