[v5,0/6,Outreachy] extend agent capability to include OS name

Message

Usman Akinyemi Feb. 14, 2025, 12:36 p.m. UTC

For debugging, statistical analysis, and security purposes, it can
be valuable for Git servers to know the operating system the clients
are using.

For example:
- A server noticing that a client is using an old Git version with
security issues on one platform, like macOS, could verify if the
user is indeed running macOS before sending a message to upgrade."
- Similarly, a server identifying a client that could benefit from
an upgrade (e.g., for performance reasons) could better customize the
message it sends to nudge the client to upgrade.

Our current agent capability is in the form of "package/version" (e.g.,
"git/1.8.3.1"). Let's extend it to include the operating system name (os)
i.e in the form "package/version os" (e.g., "git/1.8.3.1 Linux").
The operating system name is retrieved using the 'sysname' field of 
he `uname(2)` system call or its equivalent.

Including OS details in the agent capability simplifies implementation,
maintains backward compatibility, avoids introducing a new capability,
encourages adoption across Git-compatible software, and enhances
debugging by providing complete environment information without affecting
functionality.

Note that, due to differences between `uname(1)` (command-line
utility) and `uname(2)` (system call) outputs on Windows,
`transfer.advertiseOSVersion` is set to false on Windows during
testing. See the message part of patch 5/6 for more details.

My mentor, Christian Couder, sent a previous patch series about this
before. You can find it here
https://lore.kernel.org/git/[e-mail]/

Changes since v4
================
 - Remove the implementation of transfer.advertiseOSInfo config. 
 - Update the documentation.
 - Move the `os_info()` function into "version.c" file.

Usman Akinyemi (6):
  version: replace manual ASCII checks with isprint() for clarity
  version: refactor redact_non_printables()
  version: refactor get_uname_info()
  version: extend get_uname_info() to hide system details
  t5701: add setup test to remove side-effect dependency
  agent: advertise OS name via agent capability

 Documentation/gitprotocol-v2.txt | 13 +++---
 builtin/bugreport.c              | 13 +-----
 t/t5701-git-serve.sh             | 26 ++++++++++--
 t/test-lib-functions.sh          |  8 ++++
 version.c                        | 69 +++++++++++++++++++++++++++++---
 version.h                        | 10 +++++
 6 files changed, 115 insertions(+), 24 deletions(-)

Range-diff versus v4:

1:  82b62c5e66 = 1:  82b62c5e66 version: replace manual ASCII checks with isprint() for clarity
2:  0a7d7ce871 = 2:  0a7d7ce871 version: refactor redact_non_printables()
3:  0187db59a4 = 3:  0187db59a4 version: refactor get_uname_info()
4:  d3a3573594 = 4:  d3a3573594 version: extend get_uname_info() to hide system details
5:  3e0e98f23d = 5:  3e0e98f23d t5701: add setup test to remove side-effect dependency
6:  67a2767026 ! 6:  bcd1130aa1 agent: advertise OS name via agent capability
    @@ Commit message
         maintains backward compatibility, avoids introducing a new capability,
         encourages adoption across Git-compatible software, and enhances
         debugging by providing complete environment information without affecting
    -    functionality.
    -
    -    Add the `transfer.advertiseOSInfo` config option to address privacy
    -    concerns. It defaults to `true` and can be changed to `false`.
    -    When `true`, both the client and server independently append their
    -    operating system name(os) to the `agent` capability value. The `agent`
    -    capability will now be in form of "package/version os" (e.g.,
    -    "git/1.8.3.1 Linux"). When `false`, the `agent` capability will be
    -    in the form of "package/version" e.g "git/1.8.3.1". The server's
    -    configuration is independent of the client's. Defaults to `true`.
    -    The operating system name is retrieved using the 'sysname' field of
    -    the `uname(2)` system call or its equivalent.
    +    functionality. The operating system name is retrieved using the 'sysname'
    +    field of the `uname(2)` system call or its equivalent.
     
         However, there are differences between `uname(1)` (command-line utility)
         and `uname(2)` (system call) outputs on Windows. These discrepancies
    @@ Commit message
         Mentored-by: Christian Couder <[e-mail]>
         Signed-off-by: Usman Akinyemi <[e-mail]>
     
    - ## Documentation/config/transfer.txt ##
    -@@ Documentation/config/transfer.txt: transfer.bundleURI::
    - transfer.advertiseObjectInfo::
    - 	When `true`, the `object-info` capability is advertised by
    - 	servers. Defaults to false.
    -+
    -+transfer.advertiseOSInfo::
    -+	When `true`, both the client and server independently append their
    -+	operating system name (os) to the `agent` capability value. The `agent`
    -+	capability will now be in form of "package/version os" (e.g.,
    -+	"git/1.8.3.1 Linux"). When `false`, the `agent` capability will be
    -+	in the form of "package/version" e.g "git/1.8.3.1". The server's
    -+	configuration is independent of the client's. Defaults to `true`.
    -
      ## Documentation/gitprotocol-v2.txt ##
     @@ Documentation/gitprotocol-v2.txt: form `agent=X`) to notify the client that the server is running version
      the `agent` capability with a value `Y` (in the form `agent=Y`) in its
    @@ Documentation/gitprotocol-v2.txt: form `agent=X`) to notify the client that the
     -"git/1.8.3.1"). The agent strings are purely informative for statistics
     -and debugging purposes, and MUST NOT be used to programmatically assume
     -the presence or absence of particular features.
    -+printable ASCII characters (i.e., the byte range 32 < x < 127), and are
    ++printable ASCII characters (i.e., the byte range 31 < x < 127), and are
     +typically of the form "package/version os" (e.g., "git/1.8.3.1 Linux")
     +where `os` is the operating system name (e.g., "Linux"). `X` and `Y` can
     +be configured using the GIT_USER_AGENT environment variable and it takes
    -+priority. If `transfer.advertiseOSInfo` is `false` on the server, the server
    -+omits the `os` from X. If it is `false` on the client, the client omits the
    -+`os` from `Y`. The `os` is retrieved using the 'sysname' field of the `uname(2)`
    ++priority. The `os` is retrieved using the 'sysname' field of the `uname(2)`
     +system call or its equivalent. The agent strings are purely informative for
     +statistics and debugging purposes, and MUST NOT be used to programmatically
     +assume the presence or absence of particular features.
    @@ Documentation/gitprotocol-v2.txt: form `agent=X`) to notify the client that the
      ls-refs
      ~~~~~~~
     
    - ## t/t5555-http-smart-common.sh ##
    -@@ t/t5555-http-smart-common.sh: test_expect_success 'git receive-pack --advertise-refs: v1' '
    - '
    - 
    - test_expect_success 'git upload-pack --advertise-refs: v2' '
    -+	printf "agent=FAKE" >agent_capability &&
    -+	if test_have_prereq WINDOWS
    -+	then
    -+		printf "\n" >>agent_capability &&
    -+		git config transfer.advertiseOSInfo false
    -+	else
    -+		printf " %s\n" $(uname -s | test_redact_non_printables) >>agent_capability
    -+	fi &&
    - 	cat >expect <<-EOF &&
    - 	version 2
    --	agent=FAKE
    -+	$(cat agent_capability)
    - 	ls-refs=unborn
    - 	fetch=shallow wait-for-done
    - 	server-option
    -
      ## t/t5701-git-serve.sh ##
     @@ t/t5701-git-serve.sh: export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
      . ./test-lib.sh
    @@ t/t5701-git-serve.sh: export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
      
     +	if test_have_prereq WINDOWS
     +	then
    -+		printf "\n" >>agent_capability &&
    -+		git config transfer.advertiseOSInfo false
    ++		printf "agent=FAKE\n" >agent_capability
     +	else
     +		printf " %s\n" $(uname -s | test_redact_non_printables) >>agent_capability
     +	fi &&
      	cat >expect.base <<-EOF &&
      	version 2
      	$(cat agent_capability)
    +@@ t/t5701-git-serve.sh: test_expect_success 'setup to generate files with expected content' '
    + test_expect_success 'test capability advertisement' '
    + 	cat expect.base expect.trailer >expect &&
    + 
    ++	if test_have_prereq WINDOWS
    ++	then
    ++		GIT_USER_AGENT=FAKE && export GIT_USER_AGENT
    ++	fi &&
    + 	GIT_TEST_SIDEBAND_ALL=0 test-tool serve-v2 \
    + 		--advertise-capabilities >out &&
    + 	test-tool pkt-line unpack <out >actual &&
    +@@ t/t5701-git-serve.sh: test_expect_success 'test capability advertisement with uploadpack.advertiseBund
    + 	    expect.extra \
    + 	    expect.trailer >expect &&
    + 
    ++	if test_have_prereq WINDOWS
    ++	then
    ++		GIT_USER_AGENT=FAKE && export GIT_USER_AGENT
    ++	fi &&
    + 	GIT_TEST_SIDEBAND_ALL=0 test-tool serve-v2 \
    + 		--advertise-capabilities >out &&
    + 	test-tool pkt-line unpack <out >actual &&
     
      ## t/test-lib-functions.sh ##
     @@ t/test-lib-functions.sh: test_trailing_hash () {
    @@ version.c
      #include "version.h"
      #include "version-def.h"
      #include "strbuf.h"
    - #include "sane-ctype.h"
    +-#include "sane-ctype.h"
      #include "gettext.h"
    -+#include "config.h"
      
      const char git_version_string[] = GIT_VERSION;
    - const char git_built_from_commit_string[] = GIT_BUILT_FROM_COMMIT;
    -@@ version.c: const char *git_user_agent_sanitized(void)
    - 
    - 		strbuf_addstr(&buf, git_user_agent());
    - 		redact_non_printables(&buf);
    -+		/* Add os name if the transfer.advertiseosinfo config is true */
    -+		if (advertise_os_info()) {
    -+			/* Add space to space character after git version string */
    -+			strbuf_addch(&buf, ' ');
    -+			strbuf_addstr(&buf, os_info_sanitized());
    -+		}
    - 		agent = strbuf_detach(&buf, NULL);
    - 	}
    - 
    -@@ version.c: int get_uname_info(struct strbuf *buf, unsigned int full)
    - 	     strbuf_addf(buf, "%s\n", uname_info.sysname);
    - 	return 0;
    +@@ version.c: const char *git_user_agent(void)
    + 	return agent;
      }
    -+
    -+const char *os_info_sanitized(void)
    + 
    ++/*
    ++  Retrieve, sanitize and cache operating system info for subsequent
    ++  calls. Return a pointer to the sanitized operating system info
    ++  string.
    ++*/
    ++static const char *os_info(void)
     +{
     +	static const char *os = NULL;
     +
    @@ version.c: int get_uname_info(struct strbuf *buf, unsigned int full)
     +	return os;
     +}
     +
    -+int advertise_os_info(void)
    -+{
    -+	static int transfer_advertise_os_info= -1;
    + const char *git_user_agent_sanitized(void)
    + {
    + 	static const char *agent = NULL;
    +@@ version.c: const char *git_user_agent_sanitized(void)
    + 
    + 		strbuf_addstr(&buf, git_user_agent());
    + 		redact_non_printables(&buf);
     +
    -+	if (transfer_advertise_os_info == -1) {
    -+		repo_config_get_bool(the_repository, "transfer.advertiseosinfo", &transfer_advertise_os_info);
    -+		/* enabled by default */
    -+		transfer_advertise_os_info = !!transfer_advertise_os_info;
    -+	}
    -+	return transfer_advertise_os_info;
    -+}
    ++		if (!getenv("GIT_USER_AGENT")) {
    ++			strbuf_addch(&buf, ' ');
    ++			strbuf_addstr(&buf, os_info());
    ++		}
    + 		agent = strbuf_detach(&buf, NULL);
    + 	}
    + 
     
      ## version.h ##
     @@
    @@ version.h: const char *git_user_agent_sanitized(void);
      */
      int get_uname_info(struct strbuf *buf, unsigned int full);
      
    -+/*
    -+  Retrieve, sanitize and cache operating system info for subsequent
    -+  calls. Return a pointer to the sanitized operating system info
    -+  string.
    -+*/
    -+const char *os_info_sanitized(void);
    -+
    -+/*
    -+  Retrieve and cache transfer.advertiseosinfo config value. Return 1
    -+  if true, 0 if false.
    -+*/
    -+int advertise_os_info(void);
     +
      #endif /* VERSION_H */