git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 4/4] fast-import: use early_scan_options() for --allow-unsafe-features

From
Christian Couder <christian.couder@gmail.com>
Date
Oct 12, 2026, 07:44 UTC
Message-ID
<20261012074413.337183-5-christian.couder@gmail.com>
In-Reply-To
<20261012074413.337183-1-christian.couder@gmail.com>

The "feature" lines at the start of the stream are processed before the command line options are parsed, so cmd_fast_import() scans its arguments early to find out if `--allow-unsafe-features` was given.

That scan doesn't know which options take their value as a separate argument, and it stops at the first argument that doesn't start with a dash. So it disagrees with parse_options(), which accepts values separated from their option by a space, for a command line like "--depth 5 --allow-unsafe-features": the scan stops at "5" and never sees the option, so unsafe "feature" commands from the stream are refused even though the option was given.

Let's fix this by using early_scan_options(), which scans the very same `struct option` array that parse_options() uses, so that both agree on which options take a value, and by marking `--allow-unsafe-features` with PARSE_OPT_EARLY so that the scan reports it.

As early_scan_options() looks up long options the same way as parse_options(), the two also agree on abbreviations now. So `--allow-unsafe` allows unsafe "feature" commands, while "--quiet --export-pack --allow-unsafe-features" refuses them, as parse_options() takes `--allow-unsafe-features` there as the value of `--export-pack-edges`. (The old scan wrongly allowed them in this case, as all the arguments start with a dash.)

The note about having to spell the option in full in the documentation is therefore removed, and so is the NEEDSWORK comment about it.

Helped-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
 Documentation/git-fast-import.adoc |  7 ------
 builtin/fast-import.c              | 37 ++++++++++++++++--------------
 t/t9300-fast-import.sh             | 29 +++++++++++++++++++++++
 3 files changed, 49 insertions(+), 24 deletions(-)
diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
index fd165e11d2..7c5900e048 100644
--- a/Documentation/git-fast-import.adoc
+++ b/Documentation/git-fast-import.adoc
@@ -65,13 +65,6 @@ Only enable this option if you trust the program generating the
 fast-import stream! This option is enabled automatically for
 remote-helpers that use the `import` capability, as they are
 already trusted to run their own code.
-+
-Note that this option has to be spelled in full, and has to appear
-before any option whose value is separated from it by a space, for
-the unsafe `feature` commands in the stream to be allowed. So
-`--allow-unsafe` or `--depth 5 --allow-unsafe-features` still refuse
-them, while `--allow-unsafe-features --depth 5` and
-`--depth=5 --allow-unsafe-features` allow them.
 
 `--signed-tags=<mode>`::
 	Specify how to handle signed tags. Behaves in the same way as
diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index fbd919982c..2e667af349 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -4120,6 +4120,18 @@ static int option_parse_quiet(const struct option *opt UNUSED,
 	return 0;
 }
 
+static int option_parse_early_allow_unsafe(const struct option *option,
+					   const char *value UNUSED,
+					   bool unset, int pos UNUSED,
+					   void *data)
+{
+	struct fast_import_state *state = data;
+
+	if (!strcmp(option->long_name, "allow-unsafe-features"))
+		state->allow_unsafe_features = !unset;
+	return 0;
+}
+
 int cmd_fast_import(int argc,
 		    const char **argv,
 		    const char *prefix,
@@ -4184,7 +4196,7 @@ int cmd_fast_import(int argc,
 		OPT_HIDDEN_GROUP(N_("Advanced")),
 		OPT_BOOL_F(0, "allow-unsafe-features", &state.allow_unsafe_features,
 			   N_("allow unsafe mark commands from the stream"),
-			   PARSE_OPT_HIDDEN | PARSE_OPT_NONEG),
+			   PARSE_OPT_HIDDEN | PARSE_OPT_NONEG | PARSE_OPT_EARLY),
 		OPT_CALLBACK_F(0, "export-pack-edges", &state, N_("file"),
 			       N_("dump edge commits to <file>"),
 			       PARSE_OPT_HIDDEN | PARSE_OPT_NONEG,
@@ -4218,23 +4230,14 @@ int cmd_fast_import(int argc,
 	 * line to override stream data). But we must do an early parse of any
 	 * command-line options that impact how we interpret the feature lines.
 	 *
-	 * NEEDSWORK: This scan only matches the exact "--allow-unsafe-features"
-	 * spelling and stops at the first argument that doesn't start with a
-	 * dash. As parse_options() below also accepts unambiguous abbreviations
-	 * and values separated by a space from their option, the two disagree
-	 * for command lines like "--allow-unsafe" or "--depth 5
-	 * --allow-unsafe-features": parse_options() accepts the option, but
-	 * this scan doesn't see it, so unsafe features from the stream are
-	 * still refused. This errs on the safe side, but should be fixed by
-	 * teaching this scan about the options that take a value.
+	 * The scan uses the same options as parse_options() below, so the
+	 * two agree on which options take a value and on abbreviations. In
+	 * case of doubt, the scan stops early, so unsafe features are then
+	 * refused rather than wrongly allowed.
 	 */
-	for (int i = 1; i < argc; i++) {
-		const char *arg = argv[i];
-		if (*arg != '-' || !strcmp(arg, "--"))
-			break;
-		if (!strcmp(arg, "--allow-unsafe-features"))
-			state.allow_unsafe_features = 1;
-	}
+	early_scan_options(argc - 1, argv + 1, fast_import_options,
+			   EARLY_SCAN_STOP_AT_NON_OPTION,
+			   option_parse_early_allow_unsafe, &state);
 
 	rc_free = mem_pool_alloc(&fi_mem_pool, cmd_save * sizeof(*rc_free));
 	for (unsigned int i = 0; i < (cmd_save - 1); i++)
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index d9de2ef0d8..85059a5385 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -2344,6 +2344,35 @@ test_expect_success 'R: export-marks options can be overridden by commandline op
 	test_path_is_missing feature-sub
 '
 
+test_expect_success 'R: --allow-unsafe-features found after a value' '
+	echo "feature import-marks-if-exists=nonexistent.marks" >input &&
+	git fast-import --allow-unsafe-features <input &&
+	git fast-import --depth=5 --allow-unsafe-features <input &&
+	git fast-import --depth 5 --allow-unsafe-features <input &&
+	git fast-import --date-format raw --allow-unsafe-features <input
+'
+
+test_expect_success 'R: --allow-unsafe-features can be abbreviated' '
+	echo "feature import-marks-if-exists=nonexistent.marks" >input &&
+	GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=false \
+	git fast-import --allow-unsafe <input
+'
+
+test_expect_success 'R: disallowed abbreviation of --allow-unsafe-features' '
+	echo "feature import-marks-if-exists=nonexistent.marks" >input &&
+	test_must_fail env GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=true \
+		git fast-import --allow-unsafe <input 2>err &&
+	test_grep "forbidden in input without --allow-unsafe-features" err
+'
+
+test_expect_success 'R: --allow-unsafe-features as the value of an abbreviation' '
+	echo "feature import-marks-if-exists=nonexistent.marks" >input &&
+	test_must_fail env GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=false \
+		git fast-import --quiet --export-pack \
+		--allow-unsafe-features <input 2>err &&
+	test_grep "forbidden in input without --allow-unsafe-features" err
+'
+
 test_expect_success 'R: catch typo in marks file name' '
 	test_must_fail git fast-import --import-marks=nonexistent.marks </dev/null &&
 	echo "feature import-marks=nonexistent.marks" |

-- 
2.56.0.116.g6de20f6092
Previous: Christian CouderNext: Cynthia Uche
Message 31 of 40 in “Standardize early option scanning to fix argument parsing bugs”
  1. 0/6 Standardize early option scanning to fix argument parsing bugsChristian Couder, Sep 2, 2026
  2. 1/6 parse-options: add early_scan_options()Christian Couder, Sep 2, 2026
  3. Junio C HamanoSep 2, 2026
  4. Christian CouderSep 23, 2026
  5. Junio C HamanoSep 23, 2026
  6. 2/6 bisect: fix "--" detection when a term name is "--"Christian Couder, Sep 2, 2026
  7. Junio C HamanoSep 2, 2026
  8. Christian CouderSep 23, 2026
  9. Junio C HamanoSep 23, 2026
  10. 3/6 rev-parse: fix "--" detection when it is an option valueChristian Couder, Sep 2, 2026
  11. 4/6 parse-options: add parse_options_takes_argument()Christian Couder, Sep 2, 2026
  12. 5/6 parse-options: build early scan options from a struct option arrayChristian Couder, Sep 2, 2026
  13. 6/6 fast-import: use early_scan_options() for --allow-unsafe-featuresChristian Couder, Sep 2, 2026
  14. Junio C HamanoSep 4, 2026
  15. Junio C HamanoSep 2, 2026
  16. Christian CouderSep 23, 2026
  17. 0/3 Standardize early option scanningChristian Couder, Sep 23, 2026
  18. 1/3 parse-options: add parse_options_takes_argument()Christian Couder, Sep 23, 2026
  19. 2/3 parse-options: add early_scan_options()Christian Couder, Sep 23, 2026
  20. Kaartic SivaraamSep 30, 2026
  21. Christian CouderOct 12, 2026
  22. 3/3 fast-import: use early_scan_options() for --allow-unsafe-featuresChristian Couder, Sep 23, 2026
  23. 0/4 Standardize early option scanningChristian Couder, Oct 12, 2026
  24. 1/4 parse-options: add parse_options_takes_argument()Christian Couder, Oct 12, 2026
  25. 2/4 parse-options: extract lookup_long_opt() from parse_long_opt()Christian Couder, Oct 12, 2026
  26. Kaartic SivaraamOct 30, 2026
  27. Christian CouderNov 2, 2026
  28. 3/4 parse-options: add early_scan_options()Christian Couder, Oct 12, 2026
  29. Kaartic SivaraamOct 30, 2026
  30. Christian CouderNov 2, 2026
  31. 4/4 fast-import: use early_scan_options() for --allow-unsafe-featuresChristian Couder, Oct 12, 2026
  32. Cynthia UcheOct 31, 2026
  33. Christian CouderNov 2, 2026
  34. Kaartic SivaraamOct 30, 2026
  35. 0/4 Standardize early option scanningChristian Couder, Nov 2, 2026
  36. 1/4 parse-options: add parse_options_takes_argument()Christian Couder, Nov 2, 2026
  37. 2/4 parse-options: extract lookup_long_opt() from parse_long_opt()Christian Couder, Nov 2, 2026
  38. 3/4 parse-options: add early_scan_options()Christian Couder, Nov 2, 2026
  39. 4/4 fast-import: use early_scan_options() for --allow-unsafe-featuresChristian Couder, Nov 2, 2026
  40. Kaartic SivaraamNov 3, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.