# The form data below was edited by tom_tyler # Perforce Workshop Jobs # # Job: The job name. 'new' generates a sequenced job number. # # Status: Job status; required field. There is no enforced or # promoted workflow for transition of jobs from one # status to another, just a set of job status values # for users to apply as they see fit. Possible values: # # open - Issue is available to be worked on. # # inprogress - Active development is in progress. # # blocked - Issue cannot be implemented for some reason. # # fixed - Fixed, optional status to use before closed. # # closed - Issue has been dealt with definitively. # # punted - Decision made not to address the issue, # possibly not ever. # # suspended - Decision made not to address the issue # in the immediate future, but noting that it may # have some merit and may be revisited later. # # duplicate - Duplicate of another issue that. # # obsolete - The need behind the request has become # overcome by events. # # Project: The project this job is for. Required. # # Severity: [A/B/C] (A is highest) Required. # # ReportedBy The user who created the job. Can be changed. # # ReportedDate: The date the job was created. Automatic. # # ModifiedBy: The user who last modified this job. Automatic. # # ModifiedDate: The date this job was last modified. Automatic. # # OwnedBy: The owner, responsible for doing the job. Optional. # # Description: Description of the job. Required. # # DevNotes: Developer's comments. Optional. Can be used to # explain a status, e.g. for blocked, punted, # obsolete or duplicate jobs. # # Component: Projects may use this optional field to indicate # which component of the project a givenjob is associated # with. # # For the SDP, the list of components is defined in: # //guest/perforce_software/sdp/tools/components.txt # # Type: Type of job [Bug/Feature]. Required. Job: SDP-67 Status: closed Project: perforce-software-sdp Severity: C ReportedBy: tom_tyler ReportedDate: 2017/04/02 17:25:57 ModifiedBy: tom_tyler ModifiedDate: 2017/10/25 06:30:53 OwnedBy: tom_tyler Description: Defend against environment hijacking of P4ENVIRO. == Background == A core benefit of the SDP is the controlled, easily understood environment, both shell environment and Perforce context settings. These configuration items are carefully controlled in the SDP's config files and environment loading mechanism in p4_vars. The new-ish (2015.1) behavior of 'p4 set' to persist values on Unix/Linux is detrimental in an SDP environment, which is designed for live production servers. That hijacking capability is perfect for what it was intended for (mainly for DVCS usage, so a 'p4 clone' can hijack an individual user's P4CONFIG setting). But I am not sure there's a need for that on the production server, and allowing it can impact reliability/robustness of core SDP operations. Folks can always set P4ENVIRO separately or unset it (defaulting to ~/.p4enviro) if they want for some non-SDP reason on the production server. Ditto for P4CONFIG. == Shell Environment Management == The p4d trigger and p4broker filter scripts inherit their shell environment from the corresponding p4d or p4broker server process. Thus, the benefit of the SDP controlled environment mechanism includes supplemental automation, such as triggers and broker filter scripts. While the shell environment can't change after the server processes start, the Perforce context settings (P4PORT, P4CLIENT, P4USER, etc.) can be changed at any time by adjusting the contents of the P4CONFIG and/or P4ENVIRO files. Historically, P4CONFIG (which has been around for a long while) hasn't been an issue with the SDP, perhaps because changing the context settings is well-understood, and/or perhaps because it requires modifying a file on disk, which seems like a "big deal" when you're working on a production server machine. Lately, I've been finding that P4ENVIRO is a bit more prone to abuse. Maybe that's because folks don't understand it or its impact well; they do a 'p4 set' command to persist values for context settings (e.g. P4CONFIG) while logged in as 'perforce' on the production server, thinking they're just affecting their own shell environment, and inadvetently busting triggers or broker scripts. An admin might do a 'p4 set' command for personal reasons in an SDP-managed environment, and unexpectedly sabotage checkpoint scripts (e.g. p4 set P4PORT=9999), since values persisted with 'p4 set' outrank those defined in the SDP's carefully controlled shell environment. P4ENVIRO outranks even P4CONFIG, and P4ENVIRO also has capability to hijack the P4CONFIG value itself. == The Goal == The goal with this proposed change is to expliclty disable the two mechanisms, P4CONFIG and P4ENVIRO, which can impact the Perforce context settings after the server process has started. The change: Simply point both P4CONFIG and P4ENVIRO to invalid paths under /dev/null. Now, one could argue that 'p4 set' is functioning as designed, and that disabling this default behavior (a feature of the product), is more unexpected than leaving it alone. I submit that in an SDP environment designed to live on production servers, this is OK, as the higher objective is stability for production environments. == Side Effects for P4CONFIG == AFAIK, the P4CONFIG file defined in the SDP is not used. It just defines a value that points to an empty file (e.g. /p4/1/.p4config), then we don't put anything into it. This change would make it impossible to put anything into it. == Side Effects for P4ENVIRO == This change would effectively disable the ability to use the 'p4 set' command to persist values in a Unix/Linux environment. The 'p4 set' command can still be used to query values. Attempts to persist values, e.g. 'p4 set P4PORT=9999', are greeted with this error: Perforce client error: Can't set registry on UNIX. open for write: /dev/null/.p4enviro: Not a directory == Scope == This job is for the Unix/Linx SDP only. The concepts applies as ready for Windows, but the mechanics are dramatically different, and considerations are different due to the reliance on the Windows registry on Windows. That entire discussion deserves a separate job. DevNotes: This job, SDP-67, was originally named job000548. [2016/07/07 ttyler]: The original title of this review was: Defend against environment hijacking of P4ENVIRO and P4CONFIG. Per review, it was decided to change behavior only for P4ENVRIO, and to leave the original SDP setting for P4CONFIG in place. The job title was changed to reflect this. Type: Feature