You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@yetus.apache.org by GitBox <gi...@apache.org> on 2022/04/19 22:03:00 UTC

[GitHub] [yetus] aw-was-here opened a new pull request, #254: YETUS-1159. fixes for CVE-2022-24765

aw-was-here opened a new pull request, #254:
URL: https://github.com/apache/yetus/pull/254

   This patch does a few things:
   
   * updates various github actions to the latest versions which help fix things on the Github-side
   * Adds a new yetus_is_container feature routine which helps us figure out if code is running in a container
   * If precommit is running in a container, set GIT_DIR and GIT_CEILING_DIRECTORIES which will prevent git from going past our source tree.  We do not set these outside of the container because there is an assumption the user knows what they are doing.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@yetus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [yetus] aw-was-here merged pull request #254: YETUS-1159. fixes for CVE-2022-24765

Posted by GitBox <gi...@apache.org>.
aw-was-here merged PR #254:
URL: https://github.com/apache/yetus/pull/254


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@yetus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [yetus] ndimiduk commented on a diff in pull request #254: YETUS-1159. fixes for CVE-2022-24765

Posted by GitBox <gi...@apache.org>.
ndimiduk commented on code in PR #254:
URL: https://github.com/apache/yetus/pull/254#discussion_r854977904


##########
precommit/src/main/shell/core.d/00-yetuslib.sh:
##########
@@ -528,3 +528,65 @@ function yetus_set_trap_handler
     trap "${func} ${signal}" "${signal}"
   done
 }
+
+## @description  Determine if running in a container
+## @audience     public
+## @stability    evolving
+## @replaceable  no
+function yetus_is_container
+{
+  declare mounts
+  declare cgroups
+
+  if [[ -n "${YETUS_CONTAINER_STATE}" ]]; then
+    if [[ "${YETUS_CONTAINER_STATE}" == "true" ]]; then
+      return 0
+    fi
+    return 1
+  fi
+
+  if [[ -f /.dockerenv ]]; then
+    YETUS_CONTAINER_STATE=true
+    return 0
+  fi
+
+  if [[ -n "${container}" ]]; then
+    YETUS_CONTAINER_STATE=true
+    return 0
+  fi
+
+  if [[ -d /proc/self/mountinfo ]]; then

Review Comment:
   Is the order of these checks important? Is it better to start with one proc mount vs the other?



##########
precommit/src/main/shell/core.d/00-yetuslib.sh:
##########
@@ -528,3 +528,65 @@ function yetus_set_trap_handler
     trap "${func} ${signal}" "${signal}"
   done
 }
+
+## @description  Determine if running in a container
+## @audience     public
+## @stability    evolving
+## @replaceable  no
+function yetus_is_container
+{
+  declare mounts
+  declare cgroups
+
+  if [[ -n "${YETUS_CONTAINER_STATE}" ]]; then
+    if [[ "${YETUS_CONTAINER_STATE}" == "true" ]]; then
+      return 0
+    fi
+    return 1
+  fi
+
+  if [[ -f /.dockerenv ]]; then
+    YETUS_CONTAINER_STATE=true
+    return 0
+  fi
+
+  if [[ -n "${container}" ]]; then

Review Comment:
   Where does `${container}` come from?



##########
precommit/src/main/shell/core.d/01-common.sh:
##########
@@ -303,6 +303,27 @@ function common_args
   USER_PLUGIN_DIR="${BASEDIR}/.yetus/plugins.d"
 }
 
+## @description  Verify that BASEDIR is a git repo
+## @description  and set some git settings
+## @audience     public
+## @stability    evolving
+## @replaceable  no
+function verify_basedir_repo
+{
+  if [[ ! -e "${BASEDIR}/.git" ]]; then
+    yetus_error "ERROR: ${BASEDIR} is not a git repo."
+    cleanup_and_exit 1

Review Comment:
   Forgive my ignorance. Is it a common pattern in our public API that functions prefixed with `verify_` can exit the process when their condition is not met?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@yetus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [yetus] aw-was-here commented on a diff in pull request #254: YETUS-1159. fixes for CVE-2022-24765

Posted by GitBox <gi...@apache.org>.
aw-was-here commented on code in PR #254:
URL: https://github.com/apache/yetus/pull/254#discussion_r855356581


##########
precommit/src/main/shell/core.d/00-yetuslib.sh:
##########
@@ -528,3 +528,65 @@ function yetus_set_trap_handler
     trap "${func} ${signal}" "${signal}"
   done
 }
+
+## @description  Determine if running in a container
+## @audience     public
+## @stability    evolving
+## @replaceable  no
+function yetus_is_container
+{
+  declare mounts
+  declare cgroups
+
+  if [[ -n "${YETUS_CONTAINER_STATE}" ]]; then
+    if [[ "${YETUS_CONTAINER_STATE}" == "true" ]]; then
+      return 0
+    fi
+    return 1
+  fi
+
+  if [[ -f /.dockerenv ]]; then
+    YETUS_CONTAINER_STATE=true
+    return 0
+  fi
+
+  if [[ -n "${container}" ]]; then
+    YETUS_CONTAINER_STATE=true
+    return 0
+  fi
+
+  if [[ -d /proc/self/mountinfo ]]; then

Review Comment:
   From the bit of playing I've done, not really.  But I should really optimize the code to use a for loop now that I think about it.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@yetus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [yetus] aw-was-here commented on pull request #254: YETUS-1159. fixes for CVE-2022-24765

Posted by GitBox <gi...@apache.org>.
aw-was-here commented on PR #254:
URL: https://github.com/apache/yetus/pull/254#issuecomment-1105469951

   Thanks for review @ndimiduk ! I think I'm going to go ahead and merge this in if only to un-break the action test (hopefully. based upon my local tests it does at least).  We can always open more issues or PRs if there is more to do. :smile: 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@yetus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [yetus] aw-was-here commented on a diff in pull request #254: YETUS-1159. fixes for CVE-2022-24765

Posted by GitBox <gi...@apache.org>.
aw-was-here commented on code in PR #254:
URL: https://github.com/apache/yetus/pull/254#discussion_r855377244


##########
precommit/src/main/shell/core.d/00-yetuslib.sh:
##########
@@ -528,3 +528,65 @@ function yetus_set_trap_handler
     trap "${func} ${signal}" "${signal}"
   done
 }
+
+## @description  Determine if running in a container
+## @audience     public
+## @stability    evolving
+## @replaceable  no
+function yetus_is_container
+{
+  declare mounts
+  declare cgroups
+
+  if [[ -n "${YETUS_CONTAINER_STATE}" ]]; then
+    if [[ "${YETUS_CONTAINER_STATE}" == "true" ]]; then
+      return 0
+    fi
+    return 1
+  fi
+
+  if [[ -f /.dockerenv ]]; then
+    YETUS_CONTAINER_STATE=true
+    return 0
+  fi
+
+  if [[ -n "${container}" ]]; then
+    YETUS_CONTAINER_STATE=true
+    return 0
+  fi
+
+  if [[ -d /proc/self/mountinfo ]]; then

Review Comment:
   Looking at this again, I realize why I didn't use a for loop: awk is involved so the quoting gets a bit involved when any of this is in an environment variable.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@yetus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [yetus] aw-was-here commented on a diff in pull request #254: YETUS-1159. fixes for CVE-2022-24765

Posted by GitBox <gi...@apache.org>.
aw-was-here commented on code in PR #254:
URL: https://github.com/apache/yetus/pull/254#discussion_r855385250


##########
precommit/src/main/shell/core.d/01-common.sh:
##########
@@ -303,6 +303,27 @@ function common_args
   USER_PLUGIN_DIR="${BASEDIR}/.yetus/plugins.d"
 }
 
+## @description  Verify that BASEDIR is a git repo
+## @description  and set some git settings
+## @audience     public
+## @stability    evolving
+## @replaceable  no
+function verify_basedir_repo
+{
+  if [[ ! -e "${BASEDIR}/.git" ]]; then
+    yetus_error "ERROR: ${BASEDIR} is not a git repo."
+    cleanup_and_exit 1

Review Comment:
   Renamed to `check_` which makes a lot more sense.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@yetus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [yetus] aw-was-here commented on a diff in pull request #254: YETUS-1159. fixes for CVE-2022-24765

Posted by GitBox <gi...@apache.org>.
aw-was-here commented on code in PR #254:
URL: https://github.com/apache/yetus/pull/254#discussion_r855358075


##########
precommit/src/main/shell/core.d/01-common.sh:
##########
@@ -303,6 +303,27 @@ function common_args
   USER_PLUGIN_DIR="${BASEDIR}/.yetus/plugins.d"
 }
 
+## @description  Verify that BASEDIR is a git repo
+## @description  and set some git settings
+## @audience     public
+## @stability    evolving
+## @replaceable  no
+function verify_basedir_repo
+{
+  if [[ ! -e "${BASEDIR}/.git" ]]; then
+    yetus_error "ERROR: ${BASEDIR} is not a git repo."
+    cleanup_and_exit 1

Review Comment:
   Great question. This one might be the only one.  I should probably rename it from verify to ... something else.. since it isn't really a verify function.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@yetus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [yetus] aw-was-here commented on a diff in pull request #254: YETUS-1159. fixes for CVE-2022-24765

Posted by GitBox <gi...@apache.org>.
aw-was-here commented on code in PR #254:
URL: https://github.com/apache/yetus/pull/254#discussion_r855355132


##########
precommit/src/main/shell/core.d/00-yetuslib.sh:
##########
@@ -528,3 +528,65 @@ function yetus_set_trap_handler
     trap "${func} ${signal}" "${signal}"
   done
 }
+
+## @description  Determine if running in a container
+## @audience     public
+## @stability    evolving
+## @replaceable  no
+function yetus_is_container
+{
+  declare mounts
+  declare cgroups
+
+  if [[ -n "${YETUS_CONTAINER_STATE}" ]]; then
+    if [[ "${YETUS_CONTAINER_STATE}" == "true" ]]; then
+      return 0
+    fi
+    return 1
+  fi
+
+  if [[ -f /.dockerenv ]]; then
+    YETUS_CONTAINER_STATE=true
+    return 0
+  fi
+
+  if [[ -n "${container}" ]]; then

Review Comment:
   From the reading I've done, lxc will set `${container}` to `lxc` and some other container technologies will also set that to ... something.  It's a terrible check, really, but what else can we do? :shrug: 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@yetus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [yetus] aw-was-here commented on a diff in pull request #254: YETUS-1159. fixes for CVE-2022-24765

Posted by GitBox <gi...@apache.org>.
aw-was-here commented on code in PR #254:
URL: https://github.com/apache/yetus/pull/254#discussion_r855385574


##########
precommit/src/main/shell/core.d/00-yetuslib.sh:
##########
@@ -528,3 +528,65 @@ function yetus_set_trap_handler
     trap "${func} ${signal}" "${signal}"
   done
 }
+
+## @description  Determine if running in a container
+## @audience     public
+## @stability    evolving
+## @replaceable  no
+function yetus_is_container
+{
+  declare mounts
+  declare cgroups
+
+  if [[ -n "${YETUS_CONTAINER_STATE}" ]]; then
+    if [[ "${YETUS_CONTAINER_STATE}" == "true" ]]; then
+      return 0
+    fi
+    return 1
+  fi
+
+  if [[ -f /.dockerenv ]]; then
+    YETUS_CONTAINER_STATE=true
+    return 0
+  fi
+
+  if [[ -n "${container}" ]]; then

Review Comment:
   I've added some comments to this code block to hopefully better describe what is going on.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@yetus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org