Skip to content

af CLI: Don't crash when AF_CONFIG points to a non-regular file#193

Merged
kaxil merged 1 commit intomainfrom
fix-af-config-non-regular-file
Apr 27, 2026
Merged

af CLI: Don't crash when AF_CONFIG points to a non-regular file#193
kaxil merged 1 commit intomainfrom
fix-af-config-non-regular-file

Conversation

@kaxil
Copy link
Copy Markdown
Contributor

@kaxil kaxil commented Apr 27, 2026

Summary

Wrappers like astro otto set AF_CONFIG=/dev/null as a "neutralize the user's global ~/.af/config.yaml" sentinel — they don't want the agent to silently inherit a stale current-instance pointer. The intent is reasonable, but /dev/null is a character device, so when ConfigManager.load() tries FileLock(/dev/null.lock), macOS rejects the O_CREAT with EPERM:

PermissionError: [Errno 1] Operation not permitted: '/dev/null.lock'

This fires for every af subcommand — af health, af config version, af dags errors, etc. — and breaks every Otto skill that shells out to af.

This change makes ConfigManager defensive against non-regular AF_CONFIG paths regardless of who set them.

Behavior

In __init__, compute _null_path = config_path.exists() and not config_path.is_file(). This catches /dev/null, directories, fifos, sockets, and other device nodes — anything where the standard read/lock/write flow can't apply.

When _null_path is true:

  • load() returns an empty AirflowCliConfig without touching the path or attempting a FileLock.
  • save() is a no-op.

CLIContext.init() already prioritizes env vars (AIRFLOW_API_URL, AIRFLOW_USERNAME, etc.) over config values, falling back to DEFAULT_AIRFLOW_URL. With an empty config, the priority chain still resolves correctly — same as if no config file existed.

Why this approach

Three options were considered:

  1. Skip the lock in load()/save() based on file type (this PR). Single point of detection in __init__, two short-circuits. Future callers automatically get the right behavior.
  2. Catch PermissionError in _load_from_config. Hides the real exception class, fragile to other EPERM sources, doesn't help save() paths.
  3. Fall back to ~/.af/config.yaml when AF_CONFIG is unusable. Defeats the original "neutralize global config" intent — exactly what the wrapper was trying to avoid.

Option 1 keeps the wrapper's semantics ("ignore global config") while removing the crash.

Gotchas

  • af instance add / delete / use against an AF_CONFIG=/dev/null ConfigManager will appear to succeed but persist nothing. This matches the wrapper's intent (don't write to ~/.af) and is unreachable in practice — these commands aren't run by the agent. Documenting here for completeness.
  • A directory used as config_path (af -c /some/dir) now silently behaves as "no config" instead of raising. Same defensible "we got told a non-regular path, treat it as null" semantics. If we ever want to surface a clearer error for accidental misuse, we can add it later — but it's not the bug this PR addresses.

Wrappers like `astro otto` set `AF_CONFIG=/dev/null` as a "neutralize
the user's global ~/.af/config.yaml" sentinel. /dev/null is a character
device, so `FileLock(/dev/null.lock)` fails with EPERM on macOS — every
`af` invocation crashes before any command logic runs:

    PermissionError: [Errno 1] Operation not permitted: '/dev/null.lock'

ConfigManager now detects when `config_path` is a non-regular file
(directory, fifo, socket, device node) and short-circuits: load()
returns an empty config, save() is a no-op. CLIContext.init() then
resolves URL/auth from env vars (AIRFLOW_API_URL etc.) or falls back
to DEFAULT_AIRFLOW_URL — the same behavior as having no config file.
Copy link
Copy Markdown
Contributor

@jlaneve jlaneve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. clean fix, well-tested, and composes correctly with #191 to give wrappers a full "no Airflow configured" semantic when both AF_CONFIG=/dev/null and AIRFLOW_API_URL=\"\" are set

filed #194 as a follow-up for the silent-save footgun on af instance add/delete/use and other config-modifying commands. not blocking this PR

@kaxil kaxil merged commit f31be09 into main Apr 27, 2026
10 checks passed
@kaxil kaxil deleted the fix-af-config-non-regular-file branch April 27, 2026 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants