The unzip command extracts 'fda' from the archive, but there's no error handling if the 'fda' binary is not found in the zip file. This could occur if the upstream feldera-binaries-*.zip file structure changes. Consider adding error checking after the unzip command to ensure the binary was successfully extracted before attempting to create the new zip file.

The script downloads and executes a binary from GitHub without verifying its integrity using checksums or signatures. While HTTPS provides transport security, consider adding checksum verification or GPG signature validation to ensure the binary hasn't been tampered with. This is especially important for a script designed to be run via curl piped to bash.

The PATH modification for fish shell uses set -gx PATH $FELDERA_INSTALL/bin $PATH which prepends the new path. However, this is inconsistent with bash/zsh which use export PATH="$FELDERA_INSTALL/bin:$PATH". The fish syntax is correct, but for consistency and to follow fish best practices, consider using fish_add_path command if available (fish 3.2.0+), or use the explicit form: set -gx PATH "$FELDERA_INSTALL/bin" $PATH with quotes for consistency.

The script modifies shell configuration files (.bashrc, .zshrc, etc.) without checking if the PATH configuration already exists. This could result in duplicate entries being added if the script is run multiple times. Consider checking if the configuration lines already exist in the file before appending them, similar to how the script checks if BIN_DIR is already in PATH at lines 141-145.

The script uses printf '%s' "$API_RESPONSE" which is good practice to avoid interpretation of escape sequences, but the sed command that follows uses [[:space:]] character class which may not be POSIX compliant in all sed implementations. Consider using a more portable pattern like [ ] (space and tab) or testing on different platforms to ensure compatibility.

In line 34, when both FDA_VERSION and FELDERA_INSTALL environment variables are provided together, the documentation example shows them on the same line. However, this usage pattern should be clarified - the user needs to ensure both variables are passed to bash. Consider adding a note or showing the alternative syntax with separate variable exports to avoid confusion.

The table formatting for supported platforms is inconsistent with typical markdown table conventions. A table with only one column and no header row could be formatted as a simple list instead. Consider using a proper table with a header or converting to a bulleted list for better readability.

The unzip command extracts 'fda' from the archive but doesn't verify if the file was successfully extracted. If the zip file is corrupted or doesn't contain a file named 'fda', the script will fail at line 131 when trying to chmod a non-existent file. Consider adding error handling after the unzip command to provide a clearer error message if extraction fails.

The URL in line 205 references 'docs/interface/cli#optional-shell-completion' but the actual documentation section header is '### Optional: Shell completion' which would generate the anchor '#optional-shell-completion'. Verify that this URL works correctly and matches the actual documentation structure.

The curl -fsSL https://feldera.com/install | bash pattern executes a remotely downloaded shell script without any integrity verification, creating a supply-chain risk where compromise of feldera.com (or the GitHub-backed script endpoint) leads to arbitrary code execution on users’ machines. An attacker who can tamper with the served installer (via DNS/BGP hijack, web server compromise, or misconfiguration) can run arbitrary commands under the user’s account as soon as they copy-paste this one-liner. Consider changing the install flow to download the script or binary first and verify it (e.g., via a pinned version, checksum, or signature) before execution, and avoid recommending curl | bash in documentation.

it's a good change, can you also drop v${{ env.CURRENT_VERSION }} from all the other artifacts (it's not necessary because in github the version is already in the URL to the artifact

(comment below can be fixed by not adding the version here)

@gz related: since we need to package fda separately, is there still a case for feldera-binaries-* that contains everything? We may release other artifacts in the observable future? (e.g. profile visualizer .html app)

makes sense you can name the other one pipeline-mnager-v.... since it's the only binary left once you remove fda

lets call it install-fda

how do you get it deployed to feldera.com?

it would probably be easier to make it docs.feldera.com/install-fda because if you just put the script in the docs folder here it will be copied/updated from feldera/feldera to docs as part of the existing CI job

I planned to separately add a page feldera.com/install that redirects the user to raw.githubusercontent.com . It is superficially nicer (and easier to remember - basically a convention) to have that URL rather than curl -fsSL docs.feldera.com/install-fda | bash, but I like the idea of hosting the script via the docs website rather than via raw.githubusercontent

it also requires a minimum GLIBC version (you should be able to find out doing readelf --version-info ./fda | grep GLIBC_

I think you can drop this now

hardcode this so it can't easily be swapped out by something malicious

I dont think you need this as the fallback, using latest already works and is supported you can just use latest in case no version is specified

it would probably be better to put it in an existing path like /usr/local/bin than having make our own path writable. how do other install scripts solve this? maybe @blp has some advice

Actually, this seems to be a convention in a lot of modern tools - e.g. Bun, Claude, Flyctl etc. As a bonus, it keeps the fda (~/.feldera/bin) together with the default storage dir for pipeline-manager (~/.feldera/data)

Adding our own path is what a lot of tools do these days.

Personally I'm surprised that, for a single binary, we don't just recommend that people download that into a binary directory.

I think that this will only find the directory if it's in the middle of the path, but we add it at the beginning of the path. I think that we need to add a case to the expression to cover that, something like:

 "$(BIN_DIR)":* | *:"$(BIN_DIR)" | *:"$(BIN_DIR)":*)

There's another case like this in print_success.

This implementation actually wraps the PATH in semicolons, so these edge cases are matched automatically.

Oh you're right, I missed that.