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

Message

Usman Akinyemi Feb. 15, 2025, 3:50 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 v5
================
 - Used "-" instead of " " for seperating "version" and "os" in the agent string.

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 +-----
 connect.c                        |  2 +-
 t/t5701-git-serve.sh             | 26 ++++++++++--
 t/test-lib-functions.sh          |  8 ++++
 version.c                        | 69 +++++++++++++++++++++++++++++---
 version.h                        | 10 +++++
 7 files changed, 116 insertions(+), 25 deletions(-)

Range-diff versus v5:

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:  8878e9c9ab ! 6:  48cf946f61 agent: advertise OS name via agent capability
    @@ Commit message
     
         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").
    +    i.e in the form "package/version-os" (e.g., "git/1.8.3.1-Linux").
     
         Including OS details in the agent capability simplifies implementation,
         maintains backward compatibility, avoids introducing a new capability,
    @@ 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 31 < x < 127), and are
    -+typically of the form "package/version os" (e.g., "git/1.8.3.1 Linux")
    ++printable ASCII characters (i.e., the byte range 33 <= x <= 126), 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. The `os` is retrieved using the 'sysname' field of the `uname(2)`
    @@ Documentation/gitprotocol-v2.txt: form `agent=X`) to notify the client that the
      ls-refs
      ~~~~~~~
     
    + ## connect.c ##
    +@@ connect.c: const char *parse_feature_value(const char *feature_list, const char *feature, s
    + 					*offset = found + len - orig_start;
    + 				return value;
    + 			}
    +-			/* feature with a value (e.g., "agent=git/1.2.3") */
    ++			/* feature with a value (e.g., "agent=git/1.2.3-Linux") */
    + 			else if (*value == '=') {
    + 				size_t end;
    + 
    +
      ## 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
     +	then
     +		printf "agent=FAKE\n" >agent_capability
     +	else
    -+		printf " %s\n" $(uname -s | test_redact_non_printables) >>agent_capability
    ++		printf -- "-%s\n" $(uname -s | test_redact_non_printables) >>agent_capability
     +	fi &&
      	cat >expect.base <<-EOF &&
      	version 2
    @@ version.c
      #include "gettext.h"
      
      const char git_version_string[] = GIT_VERSION;
    -@@ version.c: const char *git_user_agent_sanitized(void)
    - 
    - 		strbuf_addstr(&buf, git_user_agent());
    - 		redact_non_printables(&buf);
    -+
    -+		if (!getenv("GIT_USER_AGENT")) {
    -+			strbuf_addch(&buf, ' ');
    -+			strbuf_addstr(&buf, os_info());
    -+		}
    - 		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(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;
     +}
    ++
    + const char *git_user_agent_sanitized(void)
    + {
    + 	static const char *agent = NULL;
    +@@ version.c: const char *git_user_agent_sanitized(void)
    + 		struct strbuf buf = STRBUF_INIT;
    + 
    + 		strbuf_addstr(&buf, git_user_agent());
    ++
    ++		if (!getenv("GIT_USER_AGENT")) {
    ++			strbuf_addch(&buf, '-');
    ++			strbuf_addstr(&buf, os_info());
    ++		}
    + 		redact_non_printables(&buf);
    + 		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(void);
     +
      #endif /* VERSION_H */

Comments

Junio C Hamano Feb. 18, 2025, 5:09 p.m. UTC | #1

Usman Akinyemi <[e-mail]> writes:

> Changes since v5
> ================
>  - Used "-" instead of " " for seperating "version" and "os" in the agent string.
>
> 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

Overall everything looks good.  I spotted just one nit in the
protocol documentation update, which I'll comment on separately.

Thanks.